<div dir="ltr">Just to clarify: All the LLVM incubator repositories have "enforce linear history" enabled.  Neither "Squash and Merge" or "Rebase and Merge" results in a Merge commit in the git history.  <div><br></div><div>Steve</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 11, 2020 at 3:32 PM Hubert Tong <<a href="mailto:hubert.reinterpretcast@gmail.com">hubert.reinterpretcast@gmail.com</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"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Sep 11, 2020 at 6:12 PM Renato Golin <<a href="mailto:rengolin@gmail.com" target="_blank">rengolin@gmail.com</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"><div dir="ltr"><div dir="ltr">On Fri, 11 Sep 2020 at 19:32, Hubert Tong via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>There seems to be a split between those who prefer to curate commits locally and present them in the PR (i.e. Method 3) as they are to be committed (i.e. squash/amend/etc from my workstation and push the result), and those who seem to feel that it is better to avoid "git rebase" and "git commit --amend" and let github handle rewriting the history with the commit.<br></div></div></blockquote><div>Are these primarily developers or reviewers with these preferences? Are the people using Method 3 handling inline comments diligently? Is GitHub still "great" if Method 1 is the model with the least surprises?</div></div></div></blockquote><div><br></div><div>I don't think there's a clear difference between developers and reviewers, we're all usually both at some point. I also tend to fluctuate between manually curating commits to not caring much at all. I think if we try to codify this with precision, no one will be satisfied. Worst still, a lot of people will end up doing "the wrong thing" by accident and we'll all get upset.</div><div><br></div><div>On my current project, we all have very different views and styles, but we reached a common consensus:</div><div>1. No master rewrite, just like LLVM. This is madness, period.</div><div>2. No branch merge commit into master.  Just like LLVM. We only thoroughly test the master branch.</div><div>3. The original author decides if they want to rebase or squash at the end.</div><div><br></div><div>We're already lenient with commits and reverts, there's no point in being tough just because we have a tool that allows us to do so.</div><div><br></div><div>We already require people to write decent commit messages. This would cover the review comments nicely: all comment reviews should be bundled in one last commit, with a tag like "address review comments" or whatever and a list of changes.</div><div><br></div><div>Of course, if some review changes the series consistently, it could give rise to a few more meaningful commits, with decent messages on their own. No rules broken there.</div><div><br></div><div>So, in a nutshell, review/approval like current phab,</div></div></div></blockquote><div>Current phab has better diff-to-diff comparison than anything I've seen on GitHub. It even deals with merge-from-master/rebase-on-master fairly well. "Reviewing like current phab" matches "Method 1" more.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> restrict to rebase/squash</div></div></div></blockquote><div></div><div>As in no final "merge". I'm fine with that.<br></div><div>I'll note that "rebase" is a departure from current phab in that a single review creates multiple commits. Not that I am opposed to such commits, but I am concerned with continual force pushes to PR branches. A "final history clean-up" after reviews are done is okay in my book.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>, let people decide, continue encouraging decent commit messages and squashing final review changes.</div></div></div></blockquote><div>From the above, I take it to mean that this "squashing" you mean last here is about rewriting the history 
(if not using GitHub's "squash and merge")

before using GitHub's "rebase and merge"?</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>cheers,</div><div>--renato</div></div></div>
</blockquote></div></div>
</blockquote></div>