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

Don't run Travis builds on master #235

Merged

Conversation

@aneeshusa
Copy link
Member

aneeshusa commented Mar 5, 2016

All commits on master are fast-forwards to merge commits that are
created by homu and already tested by Travis, so testing commits on
master duplicates the build.

Review on Reviewable

All commits on master are fast-forwards to merge commits that are
created by homu and already tested by Travis, so testing commits on
master duplicates the build.
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Mar 5, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Mar 5, 2016

📌 Commit 9cbbbf0 has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

bors-servo commented Mar 5, 2016

Testing commit 9cbbbf0 with merge 3d1ff41...

bors-servo added a commit that referenced this pull request Mar 5, 2016
…rsbergstrom

Don't run Travis builds on master

All commits on master are fast-forwards to merge commits that are
created by homu and already tested by Travis, so testing commits on
master duplicates the build.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/235)
<!-- Reviewable:end -->
@aneeshusa
Copy link
Member Author

aneeshusa commented Mar 5, 2016

If you look at https://travis-ci.org/servo/saltfs/builds, Travis also seems to run the build on the 'auto' branch twice - i.e., for a PR, it'll run the build on auto (for homu), and then after merging run it on master (which this gets rid of) and again on auto. Do you know why this happens? I'd like to remove the other duplicated build as well, but I don't want to break homu.

@bors-servo
Copy link
Contributor

bors-servo commented Mar 5, 2016

☀️ Test successful - travis

@bors-servo bors-servo merged commit 9cbbbf0 into servo:master Mar 5, 2016
1 check passed
1 check passed
homu Test successful
Details
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Mar 6, 2016

I think that @Manishearth has also been looking at some of the extra builds on auto. IIRC, it had to do with the double-push that homu does (first a force-push to make auto=master, then a push of the additional commit), as otherwise the single-force push will trigger travis but not buildbot.

However, my memory may be off...

@metajack
Copy link
Contributor

metajack commented Mar 6, 2016

r-

This was tried before and does not do what you want. Here's my email to travis support and their response from July of 2015:

Servo's repos run an auto-lander bot which waits for review
notification and then merges the PR into master on the branch 'auto'
and then if CI passes will fast forward master to auto.

This workflow causes at a minimum these builds:
1) Build of original PR commit (that travis merges)
2) Build of auto
3) Build of master when final merge happens

In order to reduce the spurious build #3 we have added:

branches:
except:
- master


to our .travis.yml files. Unfortunately this seems to have the
unintended consequence of disabling build #1 above.

Is this behavior intended?

Build #2 I assume isn't redundant in some cases, since master may have
move forward after Travis CI has passed a PR. Maybe our auto-lander,
homu (https://github.com/barosl/homu), can detect the case where
master is still the same and avoid build #2 in most cases.
Thank you for the e-mail and sorry for the issue. Unfortunately this is a general problem with a way whitelisting works - it works for both PRs and branches. We're planning to change it, although no work has been done on this front yet. I'll pass it to the team to discuss further and find a good solution.

Unfortunately we don't have any workaround for this at the moment, sorry.

So unless you have information that this behavior is now fixed, please revert this.

@Manishearth
Copy link
Member

Manishearth commented Mar 6, 2016

We could make it so that when it's not a pull request and the branch is master, the travis script is a no-op (there are environment variables).

Alternatively, turn on homu's exemption feature, which lets us skip build #2 if build #1 has the correct HEAD^1sha (only for repos with a single travis builder -- it works by not doing the auto merge in the first place, so if there are multiple builders it can't be done)

@aneeshusa
Copy link
Member Author

aneeshusa commented Mar 6, 2016

:( I didn't realize this had already been tried and doesn't work.

@metajack
Copy link
Contributor

metajack commented Mar 6, 2016

No worries. Great minds must think alike :)

bors-servo added a commit that referenced this pull request Mar 6, 2016
…r, r=metajack

Revert "Don't run Travis builds on master"

This reverts commit 9cbbbf0.

See #235 (comment) for the reasoning on the revert.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/236)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.