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.
