Message-ID: <20230904102819.54d95098@xps-13>
Date:   Mon, 4 Sep 2023 10:28:19 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
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

Hi Linus,

torvalds@...ux-foundation.org wrote on Sat, 2 Sep 2023 11:59:55 -0700:

> On Fri, 1 Sept 2023 at 09:42, Miquel Raynal <miquel.raynal@...tlin.com> wrote:
> >
> > Core MTD changes:
> > * Use refcount to prevent corruption
> > * Call external _get and _put in right order
> > * Fix use-after-free in mtd release
> > * Explicitly include correct DT includes
> > * Clean refcounting with MTD_PARTITIONED_MASTER
> > * mtdblock: make warning messages ratelimited
> > * dt-bindings: Add SEAMA partition bindings  
> 
> Ok, so the above list isa fairly reasonable, but then:
> 
> > MTD device driver changes:
> > * spear_smi: Use helper function devm_clk_get_enabled()
> > * maps: fix -Wvoid-pointer-to-enum-cast warning
> > * docg3: Remove unnecessary (void*) conversions
> > * physmap-core, spear_smi, st_spi_fsm, lpddr2_nvm, lantiq-flash, plat-ram:
> >   - Use devm_platform_get_and_ioremap_resource()  
> [...]
> 
> This is not a "summary of changes". The above is basically just a
> re-organized shortlog.
> 
> What I want a merge message to be is to be an _overview_ of what the
> merge brings in, and it's why I ask peopel to summarize what they have
> worked on in the pull request.
> 
> But when the "summary" is just a list of every single detail, it's no
> longer a summary. It doesn't give an overview of what has changed.
> It's not useful to an outsider as a way to see "this is what the merge
> brings in".
> 
> End result: I might as well just use "git shortlog", and it would
> probably be about as readable as this is. In fact, I get the strong
> feeling that this was auto-generated from something very much akin to
> "git shortlog", just edited to combine multiple commits that just did
> the same thing to several drivers.
> 
> Please - this is meant for *humans*. If it is just another form of
> "git shortlog", then the automated version is *better*, because I can
> use "git shortlog" to look at one particular driver (or a particular
> set of drivers), so having a static version of "git shortlog" that has
> been slightly munged to another format is actually _inferior_.
> 
> No, what the merge message should be is a general overview of "this is
> the big picture". Not just a list of every single change, just by
> sub-area.
> 
> For example, you list "Use devm_platform_get_and_ioremap_resource" not
> just for individual drivers (nobody cares!), you do it *twice*,
> because you've split up MTD drivers from raw NAND controller drivers.
> 
> And then you separately list "Use helper function
> devm_clk_get_optional_enabled" from that list too, _and_ you then list
> "Use devm_platform_ioremap_resource_byname()" for the brcm nand
> driver.
> 
> Not to mention individually lising "Fix alignment with open
> parenthesis" and "Fix the spacing" and "Fix wrong indentation" and
> "Fix a typo" for the Qcom driver.
> 
> See why I'm frustrated? This has been going on for some time, but it's
> gett9ing *worse*. This is absolutely ridiculous. I could try to make a
> summary of it all, but honestly, now it feels like just complete
> wasted time.
> 
> So I pulled this, looked at the "summary" in the fag, and decided that
> it's just not worth it, and unpulled it.
> 
> Please give me a *summary* of what has changed. A list of *important&
> things. Not a list of pointless typo fixes.

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.

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. But then, what about the
endless list of miscellaneous patches to fix the style, the W=1
warnings, various robot complains... 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?

I believe among the load of PR you receive there must be other
subsystems than mtd which receive a lot of miscellaneous changes like
that, don't hesitate to share a couple which look useful yet
concise enough to you.

Thanks anyway for pulling.

Kind regards,
Miquèl

Powered by blists - more mailing lists