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

Bring Go current in Travis #837

Merged
merged 1 commit into from
Mar 24, 2019
Merged

Bring Go current in Travis #837

merged 1 commit into from
Mar 24, 2019

Conversation

gliptak
Copy link
Contributor

@gliptak gliptak commented Mar 22, 2019

No description provided.

Copy link
Member

@kytrinyx kytrinyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gliptak

@kytrinyx kytrinyx merged commit 1cc2fce into exercism:master Mar 24, 2019
@gliptak gliptak deleted the patch-3 branch March 24, 2019 21:21
@gliptak
Copy link
Contributor Author

gliptak commented Mar 24, 2019

Could you review the Travis settings? It seems that master is not built for new commits, only for tags?

https://travis-ci.org/exercism/cli/builds

@kytrinyx
Copy link
Member

@gliptak Yes, this is on purpose. We build only pushes to pull requests, not pushes to branches. The only direct pushes to master are when we cut a new release, which only changes the version number and the changelog. All other changes (including by maintainers) are made via pull requests.

Exercism is a very big project, with lots of activity. If we build for every push to a branch and every pull request, it means we are building twice for every push. We do not get a whole lot of free concurrent resources on Travis, so by turning off pushes to branches we are able to build a lot more commits concurrently.

(Thanks for asking... this reminded me to go in and check the settings on all the Exercism repositories, and it turned out there were a bunch that had the "build pushes to branches" setting turned on).

@gliptak
Copy link
Contributor Author

gliptak commented Mar 25, 2019

This is different than the "build flow" of other projects are interacted with, but thank you for offering the rationale.

@kytrinyx
Copy link
Member

This is different than the "build flow" of other projects are interacted with

I don't understand what you mean. Would you please elaborate?

@gliptak
Copy link
Contributor Author

gliptak commented Mar 25, 2019

@kytrinyx I would recommend always building master.

Here is a scenario, where where master build might start failing:

  • PR1 opened and PR build green
  • PR2 opened and PR build green
  • PR1 approved and committed
  • At this point PR2 no longer builds against current HEAD
  • PR2 approved and committed and now master is broken

Always building master allows rolling back PR2 and asking for rebase/corrections. Instead, all subsequent PRs might start breaking in your current model.

@kytrinyx
Copy link
Member

Yeah, we've actually had this happen, now that I think about it.

I'm going to turn on the feature that requires that a PR be up-to-date with master before merging. That will ensure that we're protected against this case.

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

Successfully merging this pull request may close these issues.

2 participants