Proposal: make squash-merging the default behavior for gitlab MRs

Johan Ouwerkerk jm.ouwerkerk at gmail.com
Sat Oct 3 13:19:19 BST 2020


On Sat, Oct 3, 2020 at 12:19 PM David Hurka <david.hurka at mailbox.org> wrote:
>
> Why should colleagues navigate through any commits, when the MR is intended to
> be squashed? Wouldn’t squash merge make it easier to review an MR?
>

No because squashing happens only when merging, i.e. *after*
reviewing. So if you review commit-by-commit you have more work and
you're more likely to comment on things which turn out to be fixed by
a later commit.

>
> I think this can not be expected from new contributors.
>
> If we reject a patch submitted a new user with “Thanks, your patch finally
> fixes this annoying bug, but we can not accept it, because I don’t like your
> git history. Please learn git first.”, that would IMHO be the first step to
> make KDE a closed community.
>

I think that the other position is more accurately stated as follows:
if a patch needs work on the git commit level but the code is fine,
perhaps the maintainer should sit down and take time to checkout that
branch locally, do their git magic until they are satisfied and then
push that result to master.
Then just leave a comment on the MR thanking people for their work,
explaining their changes got merged into master via a different route
and invite the contributor to test the new shiny to validate that
their issue is fully fixed.

A similar thing is also needed w.r.t. rebasing if for example you want
to preserve a linear history in master: you can't expect drive by
contributors to have infinite patience to check back and rebase, but
it's not unreasonable for a maintainer who insists on linear history
to shoulder that burden when they get round to merging things in.

In general both approaches would point towards contributors not
merging things on their own by default until they're familiar with the
project and made a co-maintainer. It is also more closely aligned with
the PR/MR workflow itself: note the R stands for "request" after all
:) With active maintainers, that should not be hostile to new
contributors, since you mostly care that your issue gets scratched and
not who does the scratching...

But it does require very active maintainers and it may not be a good
fit for all projects. :)

Regards,

- Johan Ouwerkerk


More information about the kde-devel mailing list