Skip to content
This repository was archived by the owner on May 7, 2021. It is now read-only.

Tweak Travis #115

Merged
merged 1 commit into from
Feb 16, 2016
Merged

Tweak Travis #115

merged 1 commit into from
Feb 16, 2016

Conversation

chadwhitacre
Copy link
Contributor

  • Only one build per-PR
  • turn off IRC notifications

cf. liberapay/postgres.py#57

@chadwhitacre
Copy link
Contributor Author

cc: @techtonik et al.

@techtonik
Copy link
Contributor

Ok for disabling IRC, but why don't build branches?

@chadwhitacre
Copy link
Contributor Author

why don't build branches?

Because then we end up with two Travis builds per pull request, because of the way GitHub does pull requests. If I understand it right, GitHub automatically makes a merge commit for each pull request (I presume this is how they can determine whether the branch merges cleanly or not). Since the master branch is involved in the merge commit, by constraining to just master on Travis, we still get a test run for each commit on each pull request.

See gratipay/gratipay.com#3187 for where we introduced this on gratipay.com.

@techtonik
Copy link
Contributor

Because then we end up with two Travis builds per pull request, because of the way GitHub does pull requests.

I look through Travis and can't see where PR commits are built on branches. Can you show an example of these two builds?

@chadwhitacre
Copy link
Contributor Author

@techtonik Notice the two checks on #116, e.g., while there's only one check here.

screen shot 2016-02-16 at 8 35 20 am

screen shot 2016-02-16 at 8 33 32 am

@chadwhitacre
Copy link
Contributor Author

See also: travis-ci/travis-ci#3241.

@chadwhitacre
Copy link
Contributor Author

Rather than test the commits from the branches the pull request is sent from, we test the merge between the origin and the upstream branch.

https://docs.travis-ci.com/user/pull-requests

That makes it sound like if you test on all branches and you test pull requests, then you get two builds per PR (one for the branch, one for the PR merge commit) ... but if you only test the master branch and you test pull requests, then you get builds for each master commit and a build for each PR merge commit.

@techtonik
Copy link
Contributor

Now I see.

@techtonik
Copy link
Contributor

Looks like Travis couldn't understand this request and hung.

@techtonik
Copy link
Contributor

Rebased.

techtonik added a commit that referenced this pull request Feb 16, 2016
@techtonik techtonik merged commit ac4a253 into master Feb 16, 2016
@techtonik techtonik deleted the travis-tweaks branch February 16, 2016 16:52
@techtonik
Copy link
Contributor

And merged!

@chadwhitacre
Copy link
Contributor Author

Yay! 💃

P.S. @techtonik When you rebase can you leave the commit hash of the previous head in a comment for future reference? Thanks. :-)

@techtonik
Copy link
Contributor

leave the commit hash of the previous head in a comment

But when I force push it, it is gone, no?

@chadwhitacre
Copy link
Contributor Author

@techtonik It's not listed on this PR anymore, but GitHub still keeps it around so we can browse it if we want to.

@techtonik
Copy link
Contributor

Ok. I'll keep this in mind.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants