<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Mar 19, 2019 at 2:00 PM Tom Stellard via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
I would like to follow up on the previous thread[1], where there was a consensus<br>
to disallow merge commits in the llvm github repository, and start a discussion<br>
about how we should enforce this policy.<br>
<br>
Unfortunately, GitHub does not provide a convenient way to fully enforce this policy.<br>
We can enforce it for pull requests, but not for direct pushes to the master branch,<br>
so we will have to come up with our own solution if we want to completely prevent<br>
merge commits.  I've spent some time looking into this and here are a few<br>
options that I've come up with (If you are a GitHub expert and have other<br>
suggestions, please let us know):<br>
<br>
1. Have either a status check or use the "Rebase and Merge" policy for pull requests<br>
to disallow merge commits.  Disable direct pushes to the master branch and update the<br>
git-llvm script to create a pull request when a developer does `git llvm push` and<br>
then have it automatically merged if there are no merge commits.<br>
<br>
2. Enforce no merge commits for pull requests, but sill allow developers to push directly<br>
to master without checking for merge requests.  This is essentially a best effort<br>
approach where we avoid having to implement our own custom work-flow for committing,<br>
while accepting the possibility that someone might accidentally push a merge commit.<br>
<br>
3. Disable direct pushes to master, don't update the git-llvm script and require all<br>
developers to use pull requests, where this policy will be enforced.<br>
<br>
Which option do you prefer?<br>
<br><br></blockquote><div><br></div><div>I don't think I have a preference regarding merge commits but having a status check where some subset of build + test takes place is a really good idea, IMO.  It would be great if it were applied for everyone but if that causes too many problems, I would settle for opt-in.  Preferably not all-or-nothing on the check. </div><div><br></div><div>Regarding the scope of the check (boldly assuming one would be in place): the more, the better (so long as it's stable and tolerable to the team).  Some dev teams that use GH have a bot that optimistically batch builds-and-tests commits and when failures inevitably happen, contributors are encouraged to triage and re-enqueue their PR for being built if they can surmise the failure is not due to their change.</div><div><br></div><div>When contributing changes upstream, it takes away some of the stress/challenge if you have some independent automaton verify that your change doesn't cause regressions.  As it stands I feel like I should be on standby for 24 to 72 hours after the commit in order to investigate/revert if my change causes someone problems.  I realize that it's prudent to limit the scope such that the checks don't create an enormous backlog.</div></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature">-Brian</div></div>