Is it possible for mergebot to auto squash any fixup! and perform the merge commit as described in (a), if so then I would vote for mergebot.
Without mergebot, I vote: (a) 0 I like squashing fixup! (b) -1 (c) +1 Most of our PRs are for focused singular changes which is why I would rather squash everything over not squashing anything On Tue, Nov 28, 2017 at 9:57 AM, Kenneth Knowles <k...@google.com> wrote: > On Tue, Nov 28, 2017 at 9:51 AM, Ben Chambers <bchamb...@google.com> > wrote: > >> One risk to "squash and merge" is that it may lead to commits that don't >> have clean descriptions -- for instance, commits like "Fixing review >> comments" will show up. If we use (a) these would also show up as separate >> commits. It seems like there are two cases of multiple commits in a PR: >> >> 1. Multiple commits in a PR that have semantic meaning (eg., a PR >> performed N steps, split across N commits). In this case, keeping the >> descriptions and performing either a merge (if the commits are separately >> valid) or squash (if we want the commits to become a single commit in >> master) probably makes sense. >> > > Keep 'em > > >> 2. Multiple commits in a PR that just reflect the review history. In this >> case, we should probably ask the PR author to explicitly rebase their PR to >> have semantically meaningful commits prior to merging. (Eg., do a rebase >> -i). >> > > Ask the author to squash 'em. > > Kenn > > >> >> On Tue, Nov 28, 2017 at 9:46 AM Kenneth Knowles <k...@google.com> wrote: >> >>> Hi all, >>> >>> James brought up a great question in Slack, which was how should we use >>> the merge button, illustrated [1] >>> >>> I want to broaden the discussion to talk about all the new capabilities: >>> >>> 1. Whether & how to use the "reviewer" field >>> 2. Whether & how to use the "assignee" field >>> 3. Whether & how to use the merge button >>> >>> My preferences are: >>> >>> 1. Use the reviewer field instead of "R:" comments. >>> 2. Use the assignee field to keep track of who the review is blocked on >>> (either the reviewer for more comments or the author for fixes) >>> 3. Use merge commits, but editing the commit subject line >>> >>> To expand on part 3, GitHub's merge button has three options [1]. They >>> are not described accurately in the UI, as they all say "merge" when only >>> one of them performs a merge. They do the following: >>> >>> (a) Merge the branch with a merge commit >>> (b) Squash all the commits, rebase and push >>> (c) Rebase and push without squash >>> >>> Unlike our current guide, all of these result in a "merged" status for >>> the PR, so we can correctly distinguish those PRs that were actually merged. >>> >>> My votes on these options are: >>> >>> (a) +1 this preserves the most information >>> (b) -1 this erases the most information >>> (c) -0 this is just sort of a middle ground; it breaks commit hashes, >>> does not have a clear merge commit, but preserves other info >>> >>> Kenn >>> >>> [1] https://apachebeam.slack.com/messages/C1AAFJYMP/ >>> >>> >>> >>> >>> >>> Kenn >>> >> >
