Message-ID: <CAHk-=wj44eeYipM1Qjvena4ZG66-a04AE8H_zMtv6V1WFXYZcQ@mail.gmail.com>
Date:   Mon, 4 Sep 2023 11:22:08 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     linux-mtd@...ts.infradead.org, Richard Weinberger <richard@....at>,
        Tudor Ambarus <Tudor.Ambarus@...aro.org>,
        Vignesh Raghavendra <vigneshr@...com>,
        Frieder Schrempf <frieder.schrempf@...tron.de>,
        Michael Walle <michael@...le.cc>,
        Pratyush Yadav <pratyush@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] mtd: Changes for v6.6-rc1

On Mon, 4 Sept 2023 at 01:28, Miquel Raynal <miquel.raynal@...tlin.com> wrote:
>
> Back in 2020, you complained about one of my pull requests with:
>
> > You didn't even mention the stm32 controller change, which seems to be
> > the biggest individual thing in here..
>
> And indeed that was a mistake on my side, but I received that comment
> as a request for a more detailed list of what had been touched. That's
> likely an over interpretation, but it lead me to be more exhaustive so
> the "you did not mention <this>" would no longer happen.

You went from one extreme to another, and what I really would want is
a nice middle ground: mention the important things, summarize the
rest, and if something is subtle, please explain it.

Now, that "something is subtle" may not even happen most of the time -
particularly in drivers - so that is probably almost never an issue.

> About your request today, I totally get why you would like something
> more meaningful, but I don't know how to do it. Sometimes I get series
> which have a goal and I could definitely try to capture that goal in
> the summary rather than listing the patches.

Exactly. If you have a series with a goal, please mention / explain
the goal - not the details of the series.

But, as you say:

> But then, what about the
> endless list of miscellaneous patches to fix the style, the W=1
> warnings, various robot complains...

Absolutely. That's generally the bulk of any subsystem, and then all
you need to do is mention it as a kind of "this is what happened".

When I complained back in 2020, it was bvecause you didn't mention
even the big changes. Although quite often "big" changes can also just
be "a lot of cleanup", so mentioning it as such is also fine.

>  Because this is what I mostly get
> currently, and I believe there is no way you'll prefer something like
> this:
> * Fix misc typos
> * Fix misc style fixes
> * Update to newer API's
> Or maybe it is as long as the patches are trivial?

Absolutely. That's _exactly_ what I want for trivial patches
(including if it's a series of 15 trivial patches that all do the same
thing to 15 different drivers).

Instead of mentioning the individual patches, just say exactly the
above kind of "Update to new helper APIs", or "Fix warnings reported
by syzbot".

Honestly, for pure "endpoint" drivers, that's kind of the expected
explanation. Drivers themselves seldom have any conceptually big
changes, and so the above kind of things is normal and expected.  So
then you have exactly that kind of  "Fix W=1 warnings" comment without
any more specificity.

Then, occasionally you have one driver that gets a lot of work because
somebody goes in and cleans up that driver in _particular_, and so if
one particular driver stands out because a vendor (or an individual)
just decided to do a lot of spring cleaning, then you might have a
much more specific "Lots of work on cleaning up the XYZ driver"
mention.

Just as an example, let's look at some of the recent driver merges I
did, and take something like SCSI where not a lot of interesting stuff
happened. The mention was just

     "Updates to the usual drivers (ufs, lpfc, qla2xxx, mpi3mr, libsas) and
      the usual minor updates and bug fixes but no significant core changes"

and that's ok. It was a lot of small patches that didn't do anything
that you'd really care about unless you had some specific interest in
a driver.

But I mention that merge message because it is worth noting that I
actually complained a bit to James about it, because that pull also
did end up having one thing that stood out if you looked at the
diffstat: it removed the UFS HPB support entirely. Nobody *really*
cares about it (because it was never used), which was James' argument
for not mentioning it, but it's the kind of thing that *does* stand
out and might be worth mentioning even if it's just a curiosity. It's
a _conceptually_ interesting part of the pull, even if it doesn't
actually matter in the real world.

So I give that merge message as an example of both a perfectly fine
thing in general, but also as an example of "yeah, it could certainly
have been better". Just to give you kind of an idea what I'm looking
for.

And don't get me wrong: sometimes I really appreciate - and want - a
lot more. *IF* there are major ABI changes, I not only want them
mentioned, I really want them explained.

They *probably* don't actually happen for the MTD subsystem very much,
if at all, but to give an example of somebody who does do that kind of
"ABI changes that can affect a lot of other maintainers", we have
things like the VFS layer that then affects multiple different
filesystems, and then that shows up in the merge messages as big
explanations. Or new system calls, or things like that, which affect
not only the internal kernel ABI, but that actually exposes new
user-space ABI. Then the explanations can be tens of lines of "this is
what's going on".

So examples from the VFS layer on *those* kinds of changes:

    git show 475d4df82719
    git show c0a572d9d32f

and no, I do not expect the MTD drivers to ever do that.

But to give a recent example of a nice middle road from a merge I did
yesterday, look at the phy pull from yesterday, or the HID updates
from Friday. They have slightly different approaches to the summary,
but both of those make me feel like they are explaining what went on
in some simple summary without bogging down in excessive detail:"

    git show db906f0ca6bb
    git show 29aa98d0fe01

Anyway, all those examples are meant as just that - *examples*. It
obviously depends entirely on what has been going on, and different
subsystems can have very different rules. And often "core changes" to
the subsystem are much more worthy of mention than some cleanup of
individual drivers.

A merge message of just a single line of "Trivial cleanups to drivers"
can be entirely acceptable. But then I do expect to just see pretty
much completely mindless one- or few-liners.

Or a merge message might be 30+ lines of explanation, but then I do
expect it to be some major new feature or re-organization that will
affect end-users or other subsystems.

So there is no one single "correct" thing.

                  Linus

Powered by blists - more mailing lists