I agree with Robert. In this case one size does not fit all. There are times, another round trip with a contributor would be frustrating to the author. Especially for new contributors. Having the option to squash and merge is useful in those cases. (For reference in the past we even helped new contributors by doing small fixes at merge time.)
On Tue, Apr 17, 2018 at 2:28 PM, Robert Bradshaw <rober...@google.com> wrote: > 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 >
