I am strongly in favor of (1); I have no strong feelings about (2); I agree on (3), but generically am not hugely concerned, so long as back-references to the original PR are maintained, which is where most of the context lives. It is nice to have the change broken up into as many individually useful parts as possible, so I wouldn't really choose (b) or (c).
Of note, (1) will not be possible if you would like another contributor to review and they have not set up their gitbox account. Notably this is always going to be the case for contributors who are not committers - we should maintain use of the "R: @reviewer" comments in those cases. On Tue, Nov 28, 2017 at 9:45 AM, Kenneth Knowles <[email protected]> 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 >
