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

Simon Glass sjg at chromium.org
Fri Dec 6 20:19:38 CET 2024


Hi Tom,

On Wed, 4 Dec 2024 at 09:27, Tom Rini <trini at konsulko.com> wrote:
>
> 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.

Here's my intention with 'code needs to land': to bias against people
saying "I don't like this; I don't care about your use case; please go
away"

>
> > > 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.

I'm going to ignore that comment as it is not helpful and
mischaracterises my contributions.

>
> > > 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.

OK

>
> > 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.

If you look at the OF_BLOBLIST thing, it is basically a NAK. I don't
see any other way to interpret it,. So several boards in my lab have
been broken for a year.

>
> > 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.

That's obviously not practical for the amount I am contributing and
the length of time it takes us to get things in. I hope it isn't a
serious suggestion!

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

Yes, a bias more towards that would be more healthy IMO. A NAK needs
to come with a full understanding of the use case, an alternative way
to meet the use case, once that doesn't involve boiling the Pacific
Ocean.

Regards,
Simon


More information about the U-Boot mailing list