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

[Quentin Schulz]

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.

Thanks,
Quentin