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

Auto Rerun Pull Request Builds when Target Branch has a new Commit? #1620

Closed
drdamour opened this Issue Nov 16, 2013 · 13 comments

Comments

Projects
None yet
8 participants
@drdamour

drdamour commented Nov 16, 2013

Suppose a pull request exists that is merging _commit1_ and _commit2_ from branch _Source_ into branch _Target_

Travis has already run the tests for the pull request and found commit1 & commit2 to be passing within the pull request.

Now someone makes _commit3_ to _Target_ that does NOT cause a merge conflict in the pull request.

This may fundamentally affect the pull request even without causing a merge conflict. If you were to checkout the PR commit and run the test suite it may now fail. Or a PR that had previously been marked as a failure may now actually pass.

This appears to be quite common in our gitflow (maybe our gitflow is just bad). As such i was surprised Travis didn't do anything about this.

What i was expecting was that Travis would determine that _commit3_ was applied to _Target_ branch it would go and look for any pull requests that are merging into _Target_ that still can be auto merged. For each found PR travis would automatically rerun the builds for that PR putting them in yellow status until their overall merge status is determined.

Is there a reason for it not to do this? Would it cause undesired behaviour? or is it just a feature that hasn't been thought of or implemented yet? Sorry if it's a duplicate.

@roidrage

This comment has been minimized.

Member

roidrage commented Nov 16, 2013

This is a good question, and we were quite deliberate in not supporting this feature for the time being.

Consider an active project with lots of pull requests and lots of activity on the main branch. 20 PRs are currently open against the main branch (rails, for example, has 403 open). When master gets updated with one commit, all 20 pull requests are updated subsequently, and this happens every time someone pushes to master. Unfortunately the overall load of builds would increase significantly, causing long delays in more important builds to go through.

While you could look at this as a deficiency, it also must be said that there is still the safety net of continuously integrating against the master branch. When a PR is merge, and it results in a broken build, this will still be caught on master, the build can then either be fixed, or the merge commit can be reverted.

We may revisit this in the future, but I just wanted to give our reasoning why we did it.

@rkh

This comment has been minimized.

Member

rkh commented Nov 16, 2013

Funnily enough, our first implementation would do exactly that and we ran into the issues described by @roidrage. Maybe we can turn this into a setting.

@rkh

This comment has been minimized.

Member

rkh commented Nov 16, 2013

Note that right now you can circumvent this shortcoming on a per pull request basis by simply restarting the last build for the PR.

@drdamour

This comment has been minimized.

drdamour commented Nov 16, 2013

thanks so much, i appreciate the insight and understand the logic behind the decision.

Any chance this is a feature of travis pro?

On a pro account, If we went ahead and built something that would link the GH service hooks to travis calls to rerun a PR build to bridge this gap ourselves, would that be ok? or would we be violating the Terms of Service?

@roidrage

This comment has been minimized.

Member

roidrage commented Nov 16, 2013

It might indeed be something we'd consider offering as an option for private repositories.

You're free to run as many builds as you'd like, you just need to take into account that this counts against the number of concurrent jobs of your subscription.

@drdamour

This comment has been minimized.

drdamour commented Nov 16, 2013

ok great. Is this the appropriate place to request travis-pro features? If so, this is the request! if not let me know where to go.

for now we'll look into working around the problem on our side.

@roidrage

This comment has been minimized.

Member

roidrage commented Nov 16, 2013

It is a good place for it :)

@mhogerheijde

This comment has been minimized.

mhogerheijde commented Aug 25, 2014

I understand that this will cause the amount of builds to significantly increase. As a pro-subscriber I'd be very interested in this becoming a setting.

Other option for me might be for a hook to do this, so we could do this ourselves, might even be for specific changes. For us, this problem has only occurred so far in our database-migration scripts, since they use a sequence-number and an arbitrary description in the filename where the sequence-number occasionally (and accidentally) gets used twice. If this happens with 2 PR's at the same time, the subsequent merge will cause a broken build.

So for us it would probably be enough to only restart builds when they have changes in the folder with database-migrations. But to be safe, I'd rather restart them all ^^

@drogus

This comment has been minimized.

Member

drogus commented Aug 25, 2014

At some point I've tried to add this as a configurable feature, but it didn't work as expected, because we didn't receive any hooks: travis-ci/travis-core@33ffbef

I didn't have time to investigate it more thoroughly and currently GitHub documentation states that we should get a pull request event on synchronize, so there is a chance it will work correctly in a form proposed in travis-ci/travis-core@33ffbef

@dregad

This comment has been minimized.

dregad commented Jan 15, 2016

you can circumvent this shortcoming on a per pull request basis by simply restarting the last build for the PR

@rkh I just tried that, but it only seems to restart the build at the original commit instead of using the PR's source branch head into consideration. Here is what I did:

Did I miss something ?

@percyhanna

This comment has been minimized.

percyhanna commented Apr 6, 2017

GitHub's new "protected branches" allows you to force PRs to be up-to-date with the target branch before being merged. However, this will require manual intervention on each PR owner every time master is updated, which seems more frustrating than it's worth. Having this as an optional feature would be preferred, so you can decide whether to flood the build queue or not. For a smaller team with 5 build slots, it wouldn't be terrible given the convenience of ensuring master never breaks accidentally.

@stale

This comment has been minimized.

stale bot commented Apr 13, 2018

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please do feel free to either reopen this issue or open a new one. We'll gladly take a look again! You can read more here: https://blog.travis-ci.com/2018-03-09-closing-old-issues

@tibdex

This comment has been minimized.

tibdex commented Jul 19, 2018

GitHub's new "protected branches" allows you to force PRs to be up-to-date with the target branch before being merged. However, this will require manual intervention on each PR owner every time master is updated, which seems more frustrating than it's worth.

I like the option to force PRs to be up-to-date, it protects the repo from "semantic conflicts". But I agree that the manual intervention involved with rebasing the PR every time master is updated is annoying. That's why I developed this GitHub App: https://github.com/tibdex/autorebase. I hope you find it useful 😉.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment