Re: [GIT PULL] PCI changes for v5.15

From: Linus Torvalds <torvalds@linux-foundation.org>
To: Bjorn Helgaas <helgaas@kernel.org>, David Miller <davem@davemloft.net>
Cc: Linux PCI <linux-pci@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Subject: Re: [GIT PULL] PCI changes for v5.15
Date: Tue, 7 Sep 2021 19:08:42 -0700	[thread overview]
Message-ID: <CAHk-=wiCgh_V-g74LE4pQKqakbiK+CM5opVtH1t2+Y3R=uH9EA@mail.gmail.com> (raw)
In-Reply-To: <20210907213943.GA822380@bjorn-Precision-5520>

On Tue, Sep 7, 2021 at 2:39 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
>   drivers/net/ethernet/broadcom/bnxt/bnxt.c
>   drivers/net/ethernet/broadcom/bnx2.c
>     Fallout from the VPD changes below.  These include both PCI core and
>     driver changes, and the driver changes got merged via the net tree and
>     then reverted so everything would be merged via the PCI tree.

Christ.

So the revert from the networking tree has basically _zero_ useful
information. It just says "revert".

David, that's not ok. The natural reaction to this situation is

  "ok, this commit was done both in the networking tree and the PCI
tree, but then the networking tree reverted it. So there must be
something wrong with it, and I should take the reverted state"

but Bjorn's comment implies that it was reverted in order to _avoid_
merge conflicts since it was also done in the PCI tree, which is pure
and utter garbage, because I end up with the merge conflict *ANYWAY*
due to the other changes, and now instead of going "ok, the PCI tree
had that same commit, all good", I have to go "ok, so the PCI tree had
the same commit, but it was reverted in the networking tree, so now I
have both sides making different changes and a very confusing merge".

Here's the thing. There's a couple of very simple and basic rules:

 (a) don't do stupid things. In particular, don't try to make my
merges easier by adding MORE crap on top of the known merge problem.

     This is not that different from "don't rebase merge conflicts
away". You're making things worse.

 (b) INDEPENDENTLY of that "don't do stupid things", the #1 rules for
_any_ commit is to give the damn reason for the commit.

     You can't just say "revert X" in a commit message. That's not a
reason. That doesn't explain ANYTHING at all.

So now I have to basically guess at what is going on.

Yes, yes, I can make fairly informed guesses from looking more
carefully at the code, looking at the *other* commit messages, and
doing something sensible. So my guesses aren't going to be about
tossing a coin. But please don't do these kinds of things!

Don't make my life "easier" by doing stupid things, and DO put a
reason for every single commit you do. Reverts aren't "oh, I'm just
turning back the clock, so no reason to say anything else".

          Linus