Send build completion notification for pull requests merged after build start #472
Conversation
mguillem
force-pushed
the
feature/push-notification-for-merged-pull-request
branch
from
789c794
to
a88d25f
Compare
| // for instance PR merged on Bitbucket since the build has been started | ||
| ItemGroup<?> grandFather = build.getParent().getParent(); | ||
| if (grandFather instanceof SCMSourceOwner) { | ||
| s = ((SCMSourceOwner) grandFather).getSCMSources().get(0); |
There was a problem hiding this comment.
If a Jenkins multibranch job has multiple SCM sources (e.g. an original repository and a fork with distinct branch names), I think this might not select the correct one.
There was a problem hiding this comment.
On the other hand, because this plugin is using the older REST API that does not distinguish between repositories, it will work anyway, as long as the repositories are on the same server.
There was a problem hiding this comment.
But a similar fix might not be valid for jenkinsci/github-checks-plugin#196 because the GitHub API Create a check run is at POST /repos/{owner}/{repo}/check-runs and presumably has to be told the correct repository.
There was a problem hiding this comment.
This fix worked for me and was quite easy to implement. A solution to be really safe would be probably to save the url to which the INPROGRESS event has been sent to reuse it at build completion.
There was a problem hiding this comment.
Not only the URL but also the credential ID.
I think this change is fine for bitbucket-branch-source-plugin, but we need something more reliable for the equivalent issue in github-checks-plugin and in the future bitbucket-server-checks-plugin.
There was a problem hiding this comment.
Can someone say whether the proposed fix can be merged (and do it) or otherwise provide feedback about what should be changed? Thanks.
|
Hi maintainers, sorry to ask again but could one of you merge this PR or tell what should be changed? Thanks. |
|
JENKINS-66620 was filed today for the problem that this PR is intended to fix. |
|
@car-roll, @bitwiseman could you have a look at this PR and merge it or provide feedback if it should be improved/changed?Thanks. |
|
I don't know currently enough around this to do a comfortable merge - maybe someone else could chip-in? What would help would to have some tests though 😅 |
@lifeofguenter can you point me on some existing tests that could help to get started here? |
|
This will not work reliably for multibranch projects that have multiple branch sources, if the plugin is ever changed to use Bitbucket Server's newer build-status REST API that needs the project and repository slugs in the URL. The newer API is better in that build statuses posted to it can be deleted later (BSERV-11393, #448 (comment)). However, even the newer API would work more reliably with this PR than without this PR. |
|
@lifeofguenter I really appreciate your activity for this plugin since a few days but it's quite depressing if you say that you don't feel qualified to review this PR. It is waiting since > 4 months and until now no maintainer has had time to have a look at it. Is there any change I can make to make you feel more comfortable with my fix? |
|
@mguillem I think you can be fair enough to give me time to review it properly or add tests for the behavior prior and expected behavior afterwards if you want an instant merge. Wdyt? |
|
@lifeofguenter I'm more than happy if you plan to look at this PR soon. Your previous comment has wrongly given me a false impression. Sorry for that. |
|
we (my company and I) would have nice benefits, if this feature will be released. May you give us a short prospect if and when work will continue? Kind regards Michael |
feature/push-notification-for-merged-pull-request # Conflicts: # src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketBuildStatusNotifications.java
|
@bitwiseman, @lifeofguenter, @car-roll could you perhaps provide some feedback here? I'm ready to ensure that the PR becomes "mergeable" again but it is only worth the effort if somebody looks at it. Thanks in advance. |
|
I think a lot of users are waiting for this PR to be merged) |
This PR fixes the problem of notifications beeing not sent to Bitbucket at the end of a build causing BitBucket to show for ever "1 Build in progress".
Steps to reproduce the problem:
Your checklist for this pull request
The provided fix searches for the SCMSource in the enclosing job if the build now contains a NullSCMSource.