nemccarthy / stash-pullrequest-builder-plugin

A Jenkins plugin for Building Stash Pull Requests
https://wiki.jenkins-ci.org/display/JENKINS/Stash+pullrequest+builder+plugin
Other
64 stars 130 forks source link

Previous merged commit used when a PR is modified #94

Open ludrao opened 8 years ago

ludrao commented 8 years ago

When a PR is updated with a new commit, sometimes the plugin uses the previous merged commit rather than the new one.

Possible cause of the issue: I understood that Stash is building the merged commit/branch (origin/pr/${pullRequestId}/merge) in the background, so I suspect that there is a bad timing condition that is if the plugin query the PR and find it to be changed, it will pull the origin/pr/${pullRequestId}/merge branch before it is actually updated with the new merge commit.

This is quite annoying as it is hard to be sure that the test are passed with the latest changes!

Other than that, thx for the plugin it is great ! :)

u3r commented 7 years ago

There are several Issues and posts concerning this problem: Main explanation: https://community.atlassian.com/t5/Bitbucket-questions/Change-pull-request-refs-after-Commit-instead-of-after-Approval/qaq-p/194702 Other links: https://confluence.atlassian.com/stashkb/pull-requests-not-reflecting-changes-pushed-to-remote-branch-after-an-upgrade-385321658.html https://issues.jenkins-ci.org/browse/JENKINS-35219

The current workaround is to enable the Build only if PR is mergeable option in this plugin. This however is no solution for all scenarios: If you set up stash so pull requests need to be approved (by n developers) the PR-Builder is never triggered.

I would propose the integration of the following workaround: Create another option (checkbox) Check Mergablility before building and request the endpoint used to check whether a PR is mergable but build regardless of reply.

u3r commented 7 years ago

I've since looked at the code and Build only if Stash reports no conflicts also triggers a check to the appropriate Stash-endpoint (see StashRepository.java@b2be6792adc2e9e3670189908b09b2331da05d30 Methods isPullRequestMergable Lines 207ff and isBuildTarget Lines 248ff) So you can restrict merging and still always build the latest commit if you check this option.

I would still recommend

fkaelberer commented 7 years ago

For anybody else stumbling over this issue: consider merging locally.