Thanks for the feedback. Sounds like there are a few takeaways from the discussion:
1. Not everyone is a git power user, squash and merge is commonly used and can't be disabled. 2. As a non-committer I should expect my commits to be squashed, rewritten, or otherwise changed at the discretion of the committer. 3. I should be breaking up separate commits into separate PRs. Andrew On Wed, Apr 18, 2018 at 12:00 PM Kenneth Knowles <[email protected]> wrote: > One thing that is available is the "allow maintainers to edit" which can > let any committer push to the PR and do all of this. It is either on by > default, or I have simply checked it in the past, since I have recently had > a maintainer push to my PR on accident :-) > > On Tue, Apr 17, 2018 at 5:57 PM Ahmet Altay <[email protected]> wrote: > >> 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 <[email protected]> >> 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 <[email protected]> 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 <[email protected]> >>> 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 <[email protected]> >>> 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 <[email protected]> >>> 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 <[email protected]> >>> 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 >>> >> >>
