search

gratipay / grtp.co

Gratipay Widgets + API
https://grtp.co/
MIT License
26 stars 19 forks source link

Tweak Travis #115

Closed chadwhitacre closed 8 years ago

chadwhitacre commented 8 years ago

cf. https://github.com/gratipay/postgres.py/pull/57

chadwhitacre commented 8 years ago

cc: @techtonik et al.

techtonik commented 8 years ago

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

chadwhitacre commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

See also: https://github.com/travis-ci/travis-ci/issues/3241.

chadwhitacre commented 8 years ago

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 commented 8 years ago

Now I see.

techtonik commented 8 years ago

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

techtonik commented 8 years ago

Rebased.

techtonik commented 8 years ago

And merged!

chadwhitacre commented 8 years ago

Yay! :dancer:

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

techtonik commented 8 years ago

leave the commit hash of the previous head in a comment

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

chadwhitacre commented 8 years ago

@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 commented 8 years ago

Ok. I'll keep this in mind.