I think the two options are useful, because we have different kinds of contributors. Sophisticated users curate their own history, create logically useful commits, build atop it, etc. and merge is by far the better option. Others have a single commit followed by any number of "lint," "fixup," and "reviewer comments" ones that should clearly be squashed, and given that it takes a round trip to ask them to squash it, it's nice to be able to do it once there's an LGTM as part of the merge. At least making this fact explicit and pointing it out in the docs may be useful. On Tue, Apr 17, 2018 at 1:43 PM Mingmin Xu <mingm...@gmail.com> wrote:
> Not strongly against `Create a merge commit`, but I use `squash and merge` by default. I understand the potential impact mentioned by Andrew, it's still a better option IMO: > 1. if a PR contains several parts, it can be documented in commit message instead of several commits; --If it's a big task, let's split it into several PRs if possible; > 2. when several PRs are changing the same file, I would ask contributor to fix it; > 3. most commits are introduced by reviewer's ask, it's not necessary to do another squash(by contributors) before merge; > On Tue, Apr 17, 2018 at 1:09 PM, Robert Burke <rob...@frantil.com> wrote: >> +1 Having made a few web commits and been frustrated by the options, anything to standardize on a single option seems good to me. >> On Tue, 17 Apr 2018 at 01:49 Etienne Chauchot <echauc...@apache.org> wrote: >>> +1 to enforce the behavior recommended in the committer guide. I usually ask the author to manually squash before committing. >>> Etienne >>> Le lundi 16 avril 2018 à 22:19 +0000, Robert Bradshaw a écrit : >>> +1, though I'll admit I've been an occasional user of the "squash and merge" button when a small PR has a huge number of small, fixup changes piled on it. >>> On Mon, Apr 16, 2018 at 3:07 PM Kenneth Knowles <k...@google.com> wrote: >>> It is no secret that I agree with this. When you don't rewrite history, distributed git "just works". I didn't realize we could mechanically enforce it. >>> Kenn >>> On Mon, Apr 16, 2018 at 2:55 PM Andrew Pilloud <apill...@google.com> wrote: >>> The Github UI provides several options for merging a PR hidden behind the “Merge pull request” button. Only the “Create a merge commit” option does what most users expect, which is to merge by creating a new merge commit. This is the option recommended in the Beam committer’s guide, but it is not necessarily the default behavior of the merge button. >>> A small cleanup PR I made was recently merged via the merge button which generated a squash merge instead of a merge commit, breaking two other PRs which were based on it. See https://github.com/apache/beam/pull/4991 >>> I would propose that we disable the options for both rebase and squash merging via the Github UI. This will make the behavior of the merge button unambiguous and consistent with our documentation, but will not prevent a committer from performing these operations from the git cli if they desire. >>> Andrew > -- > ---- > Mingmin
