Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210428230528.189146-1-pbonzini@redhat.com>
In-Reply-To: <20210428230528.189146-1-pbonzini@redhat.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sat, 1 May 2021 10:10:53 -0700
Message-ID: <CAHk-=wjMc55HFTxEiJi=3iUcFm7fBeYHUvUmP1ZFYxbbs8nfXA@mail.gmail.com>
Subject: Re: [GIT PULL] KVM, AMD PSP and ARM CoreSight changes for 5.13 merge window
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        KVM list <kvm@vger.kernel.org>,
        Mathieu Poirier <mathieu.poirier@linaro.org>,
        Marc Zyngier <maz@kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Ok, got around to this now, one comment:

On Wed, Apr 28, 2021 at 4:05 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> - the coresight/next-ETE-TRBE branch from the KVM ARM tree hasn't yet
> reached you, so I am CCing the maintainer.  Since he sent the patches
> as a pull request to Marc Zyngier (the KVM ARM maintainer) at
> https://lore.kernel.org/kvmarm/20210406224437.330939-1-mathieu.poirier@linaro.org/T/#u,
> I actually suspect that from his point of view he's done.

So the problem with this is not the code, it's the merge (and
admittedly the pull request in that case).

The totality of the merge message for the coresight pull is this:

    Merge remote-tracking branch 'coresight/next-ETE-TRBE' into
kvmarm-master/next

    Signed-off-by: Marc Zyngier <maz@kernel.org>

Can you spot the problem?

And  honestly, it's not just that merge. *Most* of the merges in this
tree have absolutely garbage commit messages. This is particularly
true of Marc's merges, but there's one from you too, with the merge
message being:

    Merge branch 'kvm-sev-cgroup' into HEAD

Guys, merges need explanations. A one-liner "I merged this" is not ok.
The reason I ask for pull requests to have explanations is exactly so
that I can write reasonable merge messages.

Pull requests need to have explanations of what they pull - not just
because it needs to go into the merge message, but because the
maintainer needs to keep track of what's happening.

And even when you merge your own topic branch, you should explain
*what* you are merging and why.

Yes, it can be some simple extra line for trivial stuff ("Fix ARM
memory slot handling"), but even when it's that simple, that extra
line should be in addition to the "this is where I merged things from"
like

    Merge branch 'kvm-arm64/memslot-fixes' into kvmarm-master/next

so sometimes you only need one extra short line as a human-readable
"this is what's going on".

But then when you have something like that commit 53648ed3f085

    Merge remote-tracking branch 'coresight/next-ETE-TRBE' into
kvmarm-master/next

that actually brings in a lot of new code, that "merge from where"
really doesn't cut it.

Put another way - just look at

    gitk 53648ed3f085^..53648ed3f085^2

and tell me that that merge message is enough for what got merged.

Because it damn well isn't.

Merges are *important*. Even if nothing goes wrong, that's where
history can get messy, and the message really tells outsides what's
going on in the big picture.

And heaven forbid that a bisect points to a merge as an issue - it's
rare, but it happens - then you really want the merge to talk about
what is going on, and what it brings in.

So please people: fix your useless merge messages.

            Linus