It seems we are all on the same page, so I am fine to keep things as is too.
IIRC, GitHub.com does not allow any *user* pre-commit hooks for security reasons. "Auto-merge when all CI completes" would be a nice feature to have, but I am not sure this is something we want to develop and maintain. Cheers, Gilles On Thu, Feb 1, 2018 at 3:44 AM, Barrett, Brian via devel <devel@lists.open-mpi.org> wrote: > >> On Jan 31, 2018, at 8:33 AM, r...@open-mpi.org wrote: >> >> >> >>> On Jan 31, 2018, at 7:36 AM, Jeff Squyres (jsquyres) <jsquy...@cisco.com> >>> wrote: >>> >>> On Jan 31, 2018, at 10:14 AM, Gilles Gouaillardet >>> <gilles.gouaillar...@gmail.com> wrote: >>>> >>>> I tried to push some trivial commits directly to the master branch and >>>> was surprised that is no more allowed. >>>> >>>> The error message is not crystal clear, but I guess the root cause is >>>> the two newly required checks (Commit email checker and >>>> Signed-off-by-checker) were not performed. >>> >>> That is probably my fault; I was testing something and didn't mean to leave >>> that enabled. Oops -- sorry. :-( >>> >>> That being said -- is it a terrible thing to require a PR to ensure that we >>> get a valid email address (e.g., not a "root@localhost") and that we have a >>> proper signed-off-by line? >> >>> >>>> /* note if the commit is trivial, then it is possible to add the following >>>> line >>>> [skip ci] >>>> into the commit message, so Jenkins will not check the PR. */ >>> >>> We've had some discussions about this on the Tuesday calls -- the point was >>> made that if you allow skipping CI for "trivial" commits, it starts you >>> down the slippery slope of precisely defining what "trivial" means. >>> Indeed, I know that I have been guilty of making a "trivial" change that >>> ended up breaking something. >>> >>> FWIW, I have stopped using the "[skip ci]" stuff -- even if I made >>> docs-only changes. I.e., just *always* go through CI. That way there's >>> never any question, and never any possibility of a human mistake (e.g., >>> accidentally marking "[skip ci]" on a PR that really should have had CI). >> >> If CI takes 30 min, then not a problem - when CI takes 6 hours (as it >> sometimes does), then that’s a different story. > > If CI takes more than about 30 minutes, something’s broke. Unfortunately, > Jenkins makes that particular problem hard to deal with (monitoring for job > length). I have some thoughts on how to make it better for the OMPI Jenkins, > but am short on implementation time. So if we have any volunteers to help > here… :) > > I have no objections to PR-only for master. The other option is pre-commit > hooks, but that’s kind of ugly. We allow “rebase and merge” if people don’t > like the merge commits on trunk. Also makes updating NEWS/README items > easier when we eventually get that workflow built (because you can change the > message later with a PR, but not a commit). > > We can also experiment with adding GitHub messages to the PR when CI > completes, if people would like an async notification... > > Brian > _______________________________________________ > devel mailing list > devel@lists.open-mpi.org > https://lists.open-mpi.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@lists.open-mpi.org https://lists.open-mpi.org/mailman/listinfo/devel
