4 ms·

Hm, I thought the idea is that the only way something will land in master is if it reviews OK and CI passes all the tests on top of the current master?

Joky 8y ago
"on top of the current master" is limiting: how do you handle concurrency? I.e. two developers kicking tests on their PR on top of current master, everything green, but they merge and the result of their merge is not building (or passing tests) anymore.
- zbentley 8y ago
  Most CI systems serialize builds on master, at least at the very end of the build pipeline. That can slow things down, but, if combined with a decently fast/thought-through build process with tears, I think the benefits of not risking untested states outweigh the costs.
  - zbentley 8y ago
    *tests, not tears, but I guess it works either way...
  - Joky 8y ago
    This is not about serializing builds on master, but builds on the PR before they get merged on master. If you have a few hours of test plans, you can't serialize every PR one after each other.
    - zbentley 8y ago
      If your testing is time consuming to the point that you can only test on your PRs and never on what master would be after each one merges, then you have to choose between somehow forcing PR authors to serialize their merges somehow (external convention or tooling or something) or not testing the code that will actually be released. The latter isn’t always the wrong choice, though.
- slobotron 8y ago
  Say two PRs get sent in by two different developers. CI builds both, passes them. Reviewer comes and approves PR#1, it is merged into master. Depending how much CI power you have, PR#2 (and any other PR in queue) can be automatically built again on top of new master to verify it wont break anything. You can do it lazily too and just re-check PR#2 once it gets approved by human - like other comment mentioned, CI coordinates and serializes all merges into master making sure master is always green.
  - Joky 8y ago
    That's not very scalable: with a team of 100 developers merging on average 100 PRs a day, you have lot of concurrency. Imagine everyone want to merge at the same time, only one can, the 99 others have to re-run the build/tests. Then only one merges and 98 others have to re-run the build tests, etc. On a project where the build/tests can take hours, this becomes difficult to justify... And then, there is the "I merged a fix into the release branch, which auto-merge into master, the fix built and tested OK in the release branch but not when propagated to master".

Joky 8y ago

"on top of the current master" is limiting: how do you handle concurrency? I.e. two developers kicking tests on their PR on top of current master, everything green, but they merge and the result of their merge is not building (or passing tests) anymore.

zbentley 8y ago
Most CI systems serialize builds on master, at least at the very end of the build pipeline. That can slow things down, but, if combined with a decently fast/thought-through build process with tears, I think the benefits of not risking untested states outweigh the costs.
- zbentley 8y ago
  *tests, not tears, but I guess it works either way...
- Joky 8y ago
  This is not about serializing builds on master, but builds on the PR before they get merged on master. If you have a few hours of test plans, you can't serialize every PR one after each other.
  - zbentley 8y ago
    If your testing is time consuming to the point that you can only test on your PRs and never on what master would be after each one merges, then you have to choose between somehow forcing PR authors to serialize their merges somehow (external convention or tooling or something) or not testing the code that will actually be released. The latter isn’t always the wrong choice, though.
slobotron 8y ago
Say two PRs get sent in by two different developers. CI builds both, passes them. Reviewer comes and approves PR#1, it is merged into master. Depending how much CI power you have, PR#2 (and any other PR in queue) can be automatically built again on top of new master to verify it wont break anything. You can do it lazily too and just re-check PR#2 once it gets approved by human - like other comment mentioned, CI coordinates and serializes all merges into master making sure master is always green.
- Joky 8y ago
  That's not very scalable: with a team of 100 developers merging on average 100 PRs a day, you have lot of concurrency. Imagine everyone want to merge at the same time, only one can, the 99 others have to re-run the build/tests. Then only one merges and 98 others have to re-run the build tests, etc. On a project where the build/tests can take hours, this becomes difficult to justify... And then, there is the "I merged a fix into the release branch, which auto-merge into master, the fix built and tested OK in the release branch but not when propagated to master".