"Squash and merge" on GitHub

[Seb via Digitalmars-d]

On Saturday, 2 April 2016 at 06:02:26 UTC, Vladimir Panteleev 
wrote:
> On Saturday, 2 April 2016 at 05:18:47 UTC, Andrei Alexandrescu 
> wrote:
>> On 4/2/16 12:36 AM, Jack Stouffer wrote:
>>> On Saturday, 2 April 2016 at 03:58:27 UTC, Daniel Murphy 
>>> wrote:
>>>> On 2/04/2016 7:28 AM, Vladimir Panteleev wrote:
>>>>>
>>>>> 4. We should use the autotester's auto-merge feature anyway.
>>>>>
>>>>
>>>> Can we disable both and force everyone to use the autotester?
>>>
>>> Yes, you can take away everyone but the auto-tester's merge 
>>> rights.
>>
>> Actually that might be a good setup. We'd need an escape hatch 
>> for a small circle, and control over the autotester. Also we'd 
>> need to make the autotester work with all repos. -- Andrei
>
> Currently it seems that the autotester uses the committer's 
> account to perform the merge (probably as an additional 
> security measure that only committers can merge via the 
> autotester). It would need some changes.

FYI: One can and should enable the CI pass protection for merging 
(at least the auto-tester itself should be included). I have used 
this for a couple of repos (libmir, dlang-tour, ...) and it makes 
one sleep "safer" ;-)
(admins still have the right to merge PRs directly if really 
needed).

[1] 
https://help.github.com/articles/enabling-required-status-checks/

> Essentially it gives people who merge a PR the option that 
> instead of creating a merge commit with one parent being 
> "master" (or the target branch) and the other being a PR 
> branch, create a single commit with all the commits from the PR 
> squashed into one, with no merge commit.

I would like to restart the discussion as over the last months I 
realized how much pain it's if (a) you are managing a PR and have 
to squash your commits from time to time and thus destroy the 
review history instead of just appending the commits  or (b) if 
you are a reviewer you always have the back-and-forth between the 
submitter to explain him how to squash his PR. Especially for 
newbies or people coming from other projects this can be quite 
confusing.

I think this back-and-forth squash cycle is pretty annoying for 
everybody, a famous example for this is the "Nazi" commit at 
Phobos:

https://github.com/dlang/phobos/pull/4662

Hence I argue that we should support squashing as an option to 
make our Github PR flow easier in many cases. An alternative 
would be to add an option to the auto-tester to squash all 
commits before merging, which would retain the additional merge 
commit, but afaict this wouldn't work with CI protection enabled.

> 1. It makes bisecting more difficult because you can no longer 
> tell which PR a commit belongs to just from the DAG.

The PR id is part of the commit message

> 2. It makes bisecting more difficult because all commits are 
> squashed into one.

I see this as an advantage, see above.

> 4. We should use the autotester's auto-merge feature anyway.

Yep, but maybe autotester can support squashed merges as an 
option.

> 5. It will create merge conflicts if the PR branch is used 
> elsewhere.

That's seldom and I am not arguing that we should use squashing 
all the time.