[PATCH] doc/develop/process.rst: Expand on the Custodian workflow

[Tom Rini]

On Fri, Jan 09, 2026 at 07:08:40PM +0100, Quentin Schulz wrote:
> Hi Tom,
> 
> On 1/9/26 6:29 PM, Tom Rini wrote:
> > As seen with a recent pull request, there have been implicit assumptions
> > about how much custodians can change patches before applying and merging
> > them. This has lead to an unfortunate situation with a contributor being
> > understandably upset about the changes.
> > 
> > To avoid this in the future, document a few things:
> > - Non-trivial changes to make something apply need to have at least
> >    [name: what] in the commit message.
> > - Only trivial and obviously correct changes can be made by the
> >    custodian, and ideally this is done in a merge commit and not the
> >    patch itself.
> > - It is the responsibility of the custodian to gather relevant tags that
> >    have been provided by the community.
> > 
> > Link: https://lore.kernel.org/u-boot/1faefbf2-e8b0-4c94-85f5-025355915e0d@gmx.de/
> > Signed-off-by: Tom Rini <trini at konsulko.com>
> > ---
> > With this, Heinrich, per E Shattow's comment on IRC, can you please
> > revert the commit in question, then apply his patch as-is, and then your
> > changes as a new patch in order to show correct history?
> > 
> > And I realize at this point the subsection of this document is a bit
> > wordy. Maybe a follow-up of restructuring this part of the document
> > would be helpful? I'm not sure what would flow better however.
> > 
> > Cc: E Shattow <e at freeshell.de>
> > Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Cc: Quentin Schulz <quentin.schulz at cherry.de>
> > ---
> >   doc/develop/process.rst | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/doc/develop/process.rst b/doc/develop/process.rst
> > index 4bfbf0eb9c63..84ab09132a0b 100644
> > --- a/doc/develop/process.rst
> > +++ b/doc/develop/process.rst
> > @@ -244,7 +244,12 @@ like this:
> >         custodian can also accept patches against older code.  It can be
> >         difficult to find the correct balance between putting too much work on
> >         the custodian or too much work on an individual submitting a patch when
> > -      something does not apply cleanly.
> > +      something does not apply cleanly. If non-trivial changes need to be made
> > +      for it to apply the custodian is expected to annotate the commit message
> > +      with ``[name: note about changes made]``. Furthermore, only trivial and
> 
> We should provide an example, I think.
> 
> commit 66be03b7ee19444b23aae3990a434a7470fc1641
> Author: Jérémie Dautheribes <jeremie.dautheribes at bootlin.com>
> Date:   Fri Nov 28 12:03:04 2025 +0100
> 
>     binman: blob_dtb: improve error message when SPL is not found
> 
>     When using binman with the '-a spl-dtb=y' flag, if the SPL blob is not
>     found, binman throws a cryptic error message:
>     binman: 'NoneType' object has no attribute 'startswith'
> 
>     Let's improve the error message to explicitly state which SPL blob is
>     missing.
>     This is particularly useful when binman is used as a standalone tool
>     outside the U-Boot source tree.
> 
>     Signed-off-by: Jérémie Dautheribes <jeremie.dautheribes at bootlin.com>
>     [trini: Add '# pragma: no cover' because coverage doesn't seem to like
>     the documentation about this error]
>     Signed-off-by: Tom Rini <trini at konsulko.com>
> 
> is a good one I think.
> 
> You also added your SoB which isn't mentioned as a requirement here. I
> believe it must, because whatever you added/changed, you must comply with
> DCO to do it.
> 
> I'm tempted to say we maybe should add a link to the original (unmerged)
> commit so that people can actually do the diff themselves if they want to?
> 
> For the above example, have
> 
> Link: https://lore.kernel.org/u-boot/20251128110304.63138-1-jeremie.dautheribes@bootlin.com/
> 
> Maybe? Not sure how much friction we want to put on custodians. There's
> probably already a lot to do :)
> 
> I would really like also that custodians *tell* people that they have merged
> the commits with changes. https://docs.u-boot.org/en/latest/develop/process.html#work-flow-of-a-custodian
> says it's only "ideally" they do for merged patches, but I really think we
> should tell people when changes are made to their patches.
> 
> > +      obviously correct changes can be made, no other changes should be made to
> > +      the patch itself. Ideally all of these changes will be in a merge commit,
> > +      and not the commit itself, but that is not always possible.
> 
> I have never been a maintainer in public spaces, and in private spaces I
> enforce a strict rebase policy so I don't have merge commits ever. I
> wouldn't know what this means (but maybe custodians do, and that's the most
> important as this isn't something contributors need to know?). How are we
> supposed to do that? Note also that not everybody know that git log -c (or
> --cc? I use -c but it seems to be showing the diff of the whole merge and
> not necessarily the conflicts?) needs to be used to see the content of a git
> merge commit (it won't show with git log -p).
> 
> Can we provide an example here of a good merge commit that highlights what
> is expected from custodians?
> 
> I cannot really comment on whether this will help custodians do the right
> thing as I am no custodian.
> 
> We need to set the expectations for contributors as well.
> 
> """
> Trivial fixes may be applied by custodians when merging your patches,
> sometimes without public notice. The operated changes are explicitly listed
> at the end of the commit log or in a merge commit by the custodian.
> """
> 
> or something like that? I'm wondering where we should put this though. https://docs.u-boot.org/en/latest/develop/process.html#review-process-git-tags
> or https://docs.u-boot.org/en/latest/develop/sending_patches.html? I think
> this kind of change should be in the same commit (or one that follows after,
> but is done now).
> 
> We can have some words about proper etiquette too? Like avoiding big series,
> waiting a bit between versions, not silently ignoring previous reviews for
> newer versions (you can disagree but you need to speak your mind publicly
> and "resolve the conflict" before sending a newer version (or at the very
> least say why something wasn't done in the cover-letter or under the ---
> section in a patch), waiting at least a week (more?) to send a ping on yet
> unreviewed patches, ... That's what's coming to mind right now, there's
> probably more to add? This is a separate topic and I don't want this to be a
> blocker for this here.

That's a lot of feedback, thank you. I think this means for v2, I need
to:
- Move most of the existing wording to another section, and reference up
  to it.
- Note that some things we have done before, really shouldn't be (in your
  good example, maybe I should have pushed back on the contributor to
  try and figure out how to convince Python it *had* coverage and not
  just pragma past it, to not have the contribution sitting out even
  logner).
- Incorporate much of the above and try and remove other assumptions I
  did make / added in my wording above.

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