Applying patches to mainline (Was: Re: [PATCH v2 0/7] efi_loader: Add support for logging to a buffer)

[Tom Rini]

On Wed, Dec 04, 2024 at 08:13:04AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 3 Dec 2024 at 18:29, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Mon, Dec 02, 2024 at 05:21:05PM -0700, Simon Glass wrote:
> >
> > > From my side I'd like to change the conversation a little, to how to
> > > land code, rather than why we should bother. "Code needs to land"
> > > should be the motto. If someone has taken the time to create
> > > something, our bias should be towards getting it in, with sufficient
> > > changes to make it fit the project. There are cases where something is
> > > just a bad idea, or should be done another way, but for people working
> > > on major features or changes, biasing towards not landing the code is
> > > just going to make them go elsewhere.
> >
> > This is the wrong approach I believe. The goal has always been and
> > continues to be to have reviewed (whenever possible, our developer
> > community is small) incremental change over time.
> 
> Yes, I agree with that, but it isn't what I said.

I don't know how else to interpret "Code needs to land". I'm not sure
what reviewed patches we have that are outstanding, but also not fairly
new. We do have patches outstanding, in general. Much of those fall in
to the categories of:
- Custodian is active, but also very busy (virtually everyone is a
  volunteer so I have a hard time getting forceful with people that have
  a large queue).
- Custodian isn't active / no direct custodian and the code hasn't been
  reviewed by anyone else.

> > Just because something
> > has been posted a number of times does not mean it's ready to be merged.
> 
> I didn't say that either.

It's an impression you give however when you repost a series less than a
day after last posting, without addressing all of the feedback.

> > Your challenge today is that on patchwork you have over 150 patches
> > covering a wide variety of topics and of which many series have
> > technically-merited feedback that needs to be addressed in a technical
> > manner.
> 
> By my count I have about 10 series in progress, with a small number of
> patches (< 10?) with pending feedback

I'm not sure about that less than 10 number, in part because it's hard
to keep track of which feedback was applied, and which not. And part
because some of those series are places where I told you I'm unsure
about the core concept but you asked and I'm giving you leeway to show
me the end result.

> that isn't effectively just a
> NAK.

To be clear, "just a NAK" isn't the whole story. Those NAKs come with
technical rationales and requests. But maybe they're just lost in the
volume of emails you have in some cases. I know for example I mentioned
a few times and places that we have tests today for booting the OS, when
you mention that we need to add a test for booting the OS. We need to
expand that test, yes.

> It isn't a particularly large number, if you add up the patches I
> do in each cycle. It is in the nature of development and code review
> that things are often not right the first time, or someone else has
> another perspective, so I cannot see how this can be reduced.

The easy way to reduce it would be to go from 10 series in progress to 1
series in progress, and finish that before picking up series number 2,
and so on. And then in the future, stop when you have more than 2 or 3
in progress at once.

But really, it feels like "Code needs to land" is another way of saying
"Just a NAK should be ignored".

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241204/a894ca92/attachment.sig>