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

Previous merged commit used when a PR is modified #94

Open
ludrao opened this issue Sep 2, 2016 · 3 comments
Open

Previous merged commit used when a PR is modified #94

ludrao opened this issue Sep 2, 2016 · 3 comments

Comments

@ludrao
Copy link

ludrao commented Sep 2, 2016

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
Copy link

u3r commented May 12, 2017

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
Copy link

u3r commented May 18, 2017

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

  • either always doing a query to this endpoint
  • or making it an explicit option (preferably the default)
  • or documenting the workaround VERY LOUDLY in the pop-up help in jenkins ;-)

@fkaelberer
Copy link

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

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

No branches or pull requests

3 participants