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/[email protected]/
Signed-off-by: Tom Rini <[email protected]>
---
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 <[email protected]>
Cc: Heinrich Schuchardt <[email protected]>
Cc: Quentin Schulz <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
[trini: Add '# pragma: no cover' because coverage doesn't seem to like
the documentation about this error]
Signed-off-by: Tom Rini <[email protected]>
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/[email protected]/
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
