On 1 Feb 2019, at 22:48, Peter Wu via cfe-dev <cfe-...@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.


lldb-dev mailing list

Reply via email to