Message-ID: <20220705220432.4mgtqeuu3civvn5l@skbuf>
Date:   Tue, 5 Jul 2022 22:04:33 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Colin Foster <colin.foster@...advantage.com>
CC:     Rob Herring <robh@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Lars Povlsen <lars.povlsen@...rochip.com>,
        Steen Hegelund <Steen.Hegelund@...rochip.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Wolfram Sang <wsa@...nel.org>,
        Terry Bowman <terry.bowman@....com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "katie.morris@...advantage.com" <katie.morris@...advantage.com>
Subject: Re: [PATCH v12 net-next 0/9] add support for VSC7512 control over SPI

On Tue, Jul 05, 2022 at 01:30:15PM -0700, Colin Foster wrote:
> > Not all that useful as a changelog. I have no idea what I told you as 
> > that was probably 100s of reviews ago. When writing changelogs for patch 
> > revisions, you need to describe what changed. And it's best to put that 
> > into the relevant patch. IOW, I want to know what I said to change so I 
> > know what I need to look at again in particular.
> > 
> > And now that I've found v11, 'suggestions from Rob' isn't really 
> > accurate as you fixed errors reported by running the tools.
> > 
> > Rob
> 
> Good point - I'll be more clear going forward.

I have to say I agree with Rob, and no, you weren't more clear in the
v13 you've just posted.

First, you need to understand that a patch set with 13 revisions is on
the long side. You can't honestly expect reviewers' attention span to
last months.

Now, ok, you're at v13 already, entropy goes forward, what can you do.

First, you can link to previous versions in the cover letter, and also
parallel series containing sub-groups of patches. This information needs
to be carried throughout. I spent too long tracking your patch set
numbering system, with change sets that sometimes cover DSA and
sometimes don't, then they sometimes fork into separate series.
I lost track, let's put it this way. I'm not an expert, but I spent my
fair share of time with VSC751X datasheets and I am theoretically aware
what this patch set is trying to do, but I'm still lost.

Then, each patch needs to contain a version history of its own, in
between the "---" marker and the git short stat. Look at other patch
sets for examples. This must contain a description of the delta compared
to the previous version, including commit message rewording.

In extreme cases of large patch sets with essentially minimal changes
from version to version, maybe even the output of "git range-diff" could
be considered to be posted in the cover letter (that's rare though, but
it might help).

Generally, what you want to avoid is changing your mind in the middle of
a long patch set, especially without traceability (without being asked
to do so). Traceability here also means including links to review
feedback asking to make a design change. It may also help if you reply
to your own patch sets stating that you've found a problem in your own
code, and that you're thinking about solving it in this or that way,
even if you don't intend to get any reply.

You may even try to ask someone whom you're not working very closely
with to proof-read your patch sets and get an honest feedback "hey, are
you even following what I'm saying here? could you summarize why I'm
making the changes I'm making, and is this series generally progressing
towards a resolve?"

You got some feedback at v11 (I believe) from Jakub about reposting too
soon. The phrasing was relatively rude and I'm not sure that you got the
central idea right. Large patch sets are generally less welcome when
submitted back to back compared to small ones, but they still need to be
posted frequent enough to not lose reviewers' attention. And small
fixups to fix a build as module are not going to make a huge difference
when reviewing, so it's best not to dig your own grave by gratuitously
bumping the version number just for a compilation fix. Again, replying
to your own patch saying "X was supposed to be Y, otherwise please go on
reviewing", may help.

Also, ordering. I don't necessarily care what changed between v1 and v2
when you post v13. So you could start with the changelog for v13 and go
back in time from there, so that reviewers don't have to scroll more and
more for each revision.

Powered by blists - more mailing lists