On 16 May 2017, at 19:42, Phil Sorber wrote:
One of the things that was accomplished at the ATS Summits over the
past
several days (in addition to clearing up many many Coverity issues)
was
getting the build system integration with GitHub fixed up. We now have
real
commit status updates from Jenkins so all those ugly comments are
gone. We
were actually getting rate limited by GitHub and it brought our build
system to a crawl. We've required all these builds status to be
passing
before the PR can be merge. So no more accidental merges or getting
confused by all the comments from Jenkins.
Which brings me to the reason for this email. We are also trying out
requiring reviews. So now you have to have one positive review for a
PR to
be mergeable.
I have 3 concerns:
- The tests are not stable enough that we don’t get spurious failures
- Knowledge of the CI system is not widespread enough that everyone can
fix it when it goes wrong
- It is common to need to fix up PRs when committing them and this
process prevents that AFAICT
While I appreciate the CI and I think it is a great improvement we
should not be preventing committers committing to master.
J