-
Notifications
You must be signed in to change notification settings - Fork 89
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
Improve Bitbucket support to sent back build statuses for pull requests #26
base: master
Are you sure you want to change the base?
Conversation
any chance of this making it in? |
Ugh, I'm having the same issue - can't have that nice feature of Bitbucket server to work! |
@rafalkasa Could you resolve the merge confilcts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR and sorry for not coming back on it in a timely fashion. Please see few comments that I think must be addressed somehow before we merge the request. There is also minor merge conflicts.
// +:refs/pull-requests/(*/merge) | ||
// https://blog.jetbrains.com/teamcity/2013/02/automatically-building-pull-requests-from-github-with-teamcity | ||
|
||
if (build.getBranch().getName().contains("merge")) { // try to find pull request only for branches with merge in the name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible NPE: build.getBranch() is nullable
ByteArrayOutputStream bos = new ByteArrayOutputStream(); | ||
entity.writeTo(bos); | ||
final String json = bos.toString("utf-8"); | ||
PullRequestInfo commitInfo = myGson.fromJson(json, PullRequestInfo.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems if there is no PullRequestInfo in the response (e.g. the branch name contained "merge" substring for some other reason), the status publishing will fail. Is that right?
|
||
private StashCommit findPullRequestCommit(@NotNull SBuild build) throws PublisherException { | ||
String apiUrl = myParams.get(Constants.STASH_BASE_URL); | ||
String projectKey = myParams.get(Constants.STASH_PROJECT_KEY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For other publishers, as well as for StashSettings::testConnection method we use GitRepositoryParser.parseRepository method to extract the repo details necessary to build the URL. It may be beneficial to introduce an explicit Commit Status Publisher settings for the same, but they have to be optional in my opinion.
@Override | ||
public void processResponse(HttpResponse response) throws HttpPublisherException, IOException { | ||
|
||
final HttpEntity entity = response.getEntity(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and below please refer to the most recent changes in Commit Status Publisher, as the way we make and process these HTTP requests has changed a bit.
vote(revision.getRevision(), msg, LogUtil.describe(build)); | ||
String commitRevision = revision.getRevision(); | ||
try { | ||
StashCommit commit = findPullRequestCommit(build); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this can be addressed easily, but at the moment we try to issue HTTP requests outside the threads where the event handlers code works, so the event handlers return as quick as possible. See postAsync(...) calls in vote(...). The findPullRequestCommit(...) call is performed in the same thread and will make buildStarted, buildFinished etc. waiting if the branch name contains "merge".
I am attempting to do the same as this PR, but I am happy with the compromise that it won't work with shallow clones (I will be using git to find the parent). Interested in the PR if I get it working? |
Please get this working. We need this as well |
I find my organization in need of this behavior. If I could find the time to actually work it on what is the etiquette of finishing this work not as the original submitter? I see there are changes requested but I'm not sure if better to start over or if there is a way I could contribute directly to this existing PR. Or is just forking the original submitter's fork and submitting a new PR with the requested changes the right thing to do? I certainly don't want to take away from the work previously accomplished but the person does appear to have abandoned the effort. |
Few weeks ago my Team start to using Bitbucket with Pull Request workflow, TeamCity can build our pull request branches but commit status unfortunately was not sent back to the Bitbucket.
I spent some time to investigate how I can find pull request commit number to sent this information correct to the Bitbucket.
Unfortunately Commit Status Publisher plugin sent commit status with last revision number which was Bitbucket automatic merge.
Changes made by me allow you to sent information back to the your Bitbucket / Stash
Plugin will recognize Pull Request branches only when Branch specification parameter in VCS Roots will be configured as below:
+:refs/pull-requests/(*/merge)
Hadi Hariri also create very good post how to prepare Pull Request build from Github but follow the information you can understand how to configure your Bitbucket project because some of concepts are the same.
Automatically Building Pull Requests from GitHub
I share this pull request with others to help resolve others the same issue as my Team had.
Look below how your Pull Request can look in Bitbucket :-)
Configuration plugin view in TeamCity: