> 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
