Date:   Wed, 8 May 2019 10:55:04 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Andreas Gruenbacher <agruenba@...hat.com>
Cc:     cluster-devel <cluster-devel@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: GFS2: Pull Request

On Wed, May 8, 2019 at 4:49 AM Andreas Gruenbacher <agruenba@...hat.com> wrote:
>
> There was a conflict with commit 2b070cfe582b ("block: remove the i
> argument to bio_for_each_segment_all") on Jens's block layer changes
> which you've already merged. I've resolved that by merging those block
> layer changes; please let me know if you want this done differently.

PLEASE.

I say this to somebody pretty much every single merge window: don't do
merges for me.

You are actually just hurting, not helping. I want to know what the
conflicts are, not by being told after-the-fact, but by just seeing
them and resolving them.

Yes, I like being _warned_ ahead of time - partly just as a heads up
to me, but partly also to show that the maintainers are aware of the
notifications from linux-next, and that linux-next is working as
intended, and people aren't just ignoring what it reports.

But I do *NOT* want to see maintainers cross-merging each others trees.

It can cause nasty problems, ranging from simply mis-merges to causing
me to not pull a tree at all because one side of the development
effort had done something wrong.

And yes, mis-merges happen - and they happen to me too. It's fairly
rare, but it can be subtle and painful when it does happen.

But (a) I do a _lot_ of merges, so I'm pretty good at them, and (b) if
_I_ do the merge, at least I know about the conflict and am not as
taken by surprise by possible problems due to a mis-merge.

And that kind of thing is really really important to me as an upstream
maintainer. I *need* to know when different subtrees step on each
others toes.

As a result, even when there's a conflict and a merge is perfectly
fine, I want to know about it and see it, and I want to have the
option to pull the maintainer trees in different orders (or not pull
one tree at all), which means that maintainers *MUST NOT* do
cross-tree merges. See?

And I don't want to see back-merges (ie merges from my upstream tree,
as opposed to merges between different maintainer trees) either, even
as a "let me help Linus, he's already merged the other tree, I'll do
the merge for him". That's not helping, that's just hiding the issue.

Now, very very occasionally I will hit a merge that is so hard that I
will go "Hmm, I really need the involved parties to sort this out".
Honestly, I can't remember the last time that happened, but it _has_
happened.

Much more commonly, I'll do the merge, but ask for verification,
saying "ok, this looked more subtle than I like, and I can't test any
of it, so can you check out my merge". Even that isn't all that
common, but it definitely happens.

There is _one_ very special kind of merge that I like maintainers
doing: the "test merge".

That test merge wouldn't be sent to me in the pull request as the
primary signed pull, but it's useful for the maintainer to do to do a
final *check* before doing the pull request, so that you as a
maintainer know what's going on, and perhaps to warn me about
conflicts.

If you do a test merge, and you think the test merge was complex, you
might then point to your resolution in the pull request as a "this is
how I would do it". But you should not make that merge be *the* pull
request.

One additional advantage of a test merge is that it actually gives a
"more correct" diffstat for the pull request. Particularly if the pull
request is for something with complex history (ie you as a maintainer
have sub-maintainers, and have pulled other peoples work), a
test-merge can get a much better diffstat. I don't _require_ that
better diffstat, though - I can see how you got the "odd" diffstat if
you don't do a test merge - but it's can be a "quality of pull
request" thing.

See what I'm saying? You would ask me to pull the un-merged state, but
then say "I noticed a few merge conflicts when I did my test merge,
and this is what I did" kind of aside.

                     Linus

Powered by blists - more mailing lists