[lldb-dev] [cfe-dev] [llvm-dev] [Github] RFC: linear history vs merge commits

[David Chisnall via lldb-dev]

On 1 Feb 2019, at 22:48, Peter Wu via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> On caveat is that the test coverage would be limited or else the build
> times would be very long. There are quite some builders for various
> projects (llvm, cfe, compiler-rt, etc.) on a lot of different platforms
> and targets (Linux, macOS, Windows, Android, etc.).
> 
> With limited resources, Arthur's suggestion seems workable to me:
> - Enable pre-commit testing of few configurations (in parallel).
> - Encourage developers to wait for tests to pass before pushing changes.
> - Do not block hard to avoid a situation where unrelated changes
>  (commits or build environment) cause failures.

GitHub does let you block merges (by anyone who isn’t an administrator) of PRs that don’t meet certain requirements.  For one project, we have it set up so that you need to have at least one reviewer saying approved, you have to have signed the CLA, and you have to pass all of the CI checks.  It’s then easy to set up automatic merging when all of the prerequisites are met (you can get a notification and process it automatically). We allow self approval for small changes (not automatically enforced, this is a judgement call, but you can remove people who abuse it from the team quite easily and then they can’t approve changes).

I’ve found it leads to a very nice workflow: developers aren’t in the loop for merges, they just prepare something that they think works, submit it, and get on with something else.  If you’re lucky, someone approves it and you pass CI first go, then everything is fine.  Reviewers can wait until CI is finished and not bother looking for things that are going to break.  If the change introduces new tests, reviewers know that those tests are passing.  If you broke a platform that you don’t test on locally, you get notified but no one else is spammed with breakage reports.  Once you’ve fixed it, you get a review, and make any changes.  The master branch is always buildable and passing CI.  If your changes break CI, you don’t have a race to fix things before someone reverts your change, you just fix things in the PR and wait for it to be automatically merged.

David