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

Use unified build status key with exclude origin PR Branches strategy #172

Merged

Conversation

JeanChristopheMorinPerso
Copy link
Contributor

…r filter is used.

Little improvement in the way the build status key is named when the Discover branch option is set to Exclude branches that are also filed as PRs. With these changes, the build key for a branch that then goes into PR will stay the same (<project_name>/<branch_name>), like it is for any branch right now.

I tested all possible options of the discover branch and my change is really only affecting the build key when Exclude branches that are also filed as PRs is chosen.

This should hopefully fix #160. I also hope it will be more solid than #147.

I'm not a Java developer, and so I did my best with the knowledge I have to keep the code style. Let me know if there is anything to change.

@jetersen
Copy link
Member

@JeanChristopheMorinPerso I think it would be better to just pass the bitbucketsource that is already defined in sendNotifications method and pass to createStatus

@jetersen
Copy link
Member

I have gone ahead and added my suggestion, please feel free to review it :)

@JeanChristopheMorinPerso
Copy link
Contributor Author

Thanks a lot @Casz , it looks much nicer now. I'll test the changes later today (even if not much change from my changes). I really appreciate the time you took to improve the PR! :)

@JeanChristopheMorinPerso
Copy link
Contributor Author

I'll fix the build. Looks like one import is wrongly ordered.

@jetersen
Copy link
Member

@JeanChristopheMorinPerso fixed 👍

@JeanChristopheMorinPerso
Copy link
Contributor Author

Looks like you are faster than me 😄 Tested the latest changes and everything looks good 👍

@JeanChristopheMorinPerso
Copy link
Contributor Author

@Casz Do you a time frame for when the next release will happen? Don't want to be pushy or anything... It's more by curiosity.

@jetersen
Copy link
Member

jetersen commented Mar 3, 2019

Would be great if I could land this, #173 and #174 together in next release 😄

@jetersen
Copy link
Member

I'll see if I can land the PRs myself in the weekend

@JeanChristopheMorinPerso
Copy link
Contributor Author

Thanks!

@timshadel
Copy link

@Casz Any update on getting this PR in and released? It'd really help us out! 😄

@jetersen
Copy link
Member

jetersen commented Apr 4, 2019

Time did not allow me to work on this :|
Since the authors PR of #173 and #174 is not willing to listen to PR reviews, I have to do the work myself.

@jetersen jetersen force-pushed the common_build_status_key branch from e2920f4 to 7f5c11e Compare April 6, 2019 20:05
@jetersen jetersen changed the title Use unified build status key when ExcludeOriginPRBranchesSCMHeadFilte… Use unified build status key with exclude origin PR Branches strategy Apr 7, 2019
@jetersen jetersen merged commit 0e116e1 into jenkinsci:master Apr 7, 2019
@JeanChristopheMorinPerso
Copy link
Contributor Author

Thank you @Casz ! I really appreciate the time you took to improve the work I initially did on this PR :)

I'm sure a lot of other people will be happy to see this feature finally released.

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

Successfully merging this pull request may close these issues.

3 participants