Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JENKINS-65741: Implement a new option that will allow to include jenkins url to the commit status #454

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dee-kryvenko
Copy link
Contributor

@dee-kryvenko dee-kryvenko commented May 28, 2021

This PR will add a new option to the existing commit status trait. Enabling this option will add Jenkins URL to the commit status key and description to differentiate notifications from different systems.

I initially wanted to include URL into the build status name, but at least Bitbucket Server does not making status popup windows wider to accommodate that. The end of the name getting shrined into .... I thought it is more important to see info about the job atop, so I included URL to the description instead of the name.

https://issues.jenkins.io/browse/JENKINS-65741

I could not find any existing tests for the commit statuses, and I am not really up to the challenge to cover it all with tests in the scope of this PR, so no tests in this PR. I did tested it locally on my Bitbucket Server instance.

Your checklist for this pull request

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

screencapture-stash-mtvi-projects-CDCNRY-repos-vmn-dummy-canary-app-pull-requests-12-builds-2021-06-21-19_12_47

return DisplayURLProvider.get().getRoot();
}

private static String getRunURL(@NonNull Run<?, ?> build) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method was confusing - it was returning job url but saying it was returning root url, and since I needed a new method that would return a root url - I had to rename this.

This method also never actually uses build and cfg so I am not sure what's the purpose of everything in there except for the last line.

try {
url = getRootURL(build);
rootUrl = getRootURL();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Root URL does not need a checkURL. There is no requirements for the text fields it is used in, and we want to keep whatever the user have put in there.

@@ -231,6 +246,10 @@ private static String getBuildKey(@NonNull Run<?, ?> build, String branch,
key = build.getParent().getFullName(); // use the job full name as the key for the status
}

if (includeJenkinsURL) {
key = rootUrl + "/" + key;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be double / depending on the user input - but I figured it doesn't makes any difference, the string is just a seed for a hash - as long as it's consistent, we're good.

@bitwiseman bitwiseman self-requested a review June 10, 2021 14:35
@ssc327
Copy link
Contributor

ssc327 commented Jun 13, 2021

Is it possible to show a screenshot of how this looks on Bitbucket?

@dee-kryvenko
Copy link
Contributor Author

dee-kryvenko commented Jun 13, 2021

Is it possible to show a screenshot of how this looks on Bitbucket?

@ssc327 I just attached a screenshot above. The top entry coming from a Jenkins instance with this option enabled.

@dee-kryvenko
Copy link
Contributor Author

Anyone?

@dee-kryvenko
Copy link
Contributor Author

Actually I was just testing it on v7.13.0 and their new re-designed build status tab just ignoring description field completely. It's just nowhere to be seen. And it's sill there in the API doc. Very mature perfectly backward compatible change... well done Atlassian...
I updated the PR to include url into the status name. Attached is a new screenshot how it looks like in the new tab now - width doesn't seem to be a concern anymore.

….com:dee-kryvenko/bitbucket-branch-source-plugin into feature/JENKINS-65741-add-key-override-trait
@dee-kryvenko
Copy link
Contributor Author

FYI there is a very interesting discussion going on here https://jira.atlassian.com/browse/BSERV-12817 relatively to this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants