Hi Ralph,
On 11/5/26 3:33 PM, Ralph Siemsen wrote:
On Thu, Nov 05, 2026 at 12:00:27PM +0100, Quentin Schulz wrote:
On 11/4/26 8:48 PM, Ralph Siemsen wrote:
+Of course there are sometimes cases where a large patch series is okay:
+
+* when the changes are all in a specific area: for example when
fixing a bug
+ in a driver, but also doing some refactoring or cleanup in the
same code
issue: This is typically the things we don't want to see I believe.
Separate bugs from refactoring, cleanup, and new features. The
argument is: we want the bug to be fixed as soon as possible, but we
*reaaaaally* like to merge full series at once, so having this kind of
changes mixed in a series means it's likely we'll either ask the
contributor to split the series, or if the non-bug-fixing part of the
series is controversial or not in a good state yet, unnecessarily
postpone the merging of the bug fix.
Maybe I should have said: "*directly related* refactoring or cleanup"?
Ideally a bug fix should be its own patch, and certainly it should not
include any new features. But sometimes a bit of refactor/cleanup helps
to make a bug fix easier to implement and/or understand.
Sometimes it's tempting to squeeze some refactoring that isn't actually
necessary. I'm sure there are exceptions, but the more there is in
context for a bug fix to be merged, the longer it'll take for the bug to
be fixed, that's unfortunately the reality. We can strongly
encourage/discourage people to do something, but there are of course
always exceptions. Best judgment applies, but everybody has a different
one and it gets trained over time :)
Something along the lines of "In the event a refactoring drastically
simplifies bug fixing, it may be part of the same series, but it should
be understood that this means the review will likely take much longer
than if it was the bug fix alone in the series".
+* when there are unavoidable dependencies between patches: keeping them
+ together avoids having to explain how multiple series are dependent
thought: People usually struggle to detect the *actual* dependencies
and are usually leaning too much on "I need those two drivers to be
merged for the board to work" for example. We've seen this more
recently with a full SoC bring-up being sent as one series. Yes, the
SoC is unusable without all the minimally required drivers, but each
individual driver can and should be sent a separate series. I'm not
sure how we can word this better, maybe providing a real-life example
would be nice. This can always be improved later on, so not a blocker.
Having struggled with this myself, while I do agree with the idea of
using separate series, it does complicate the development process. It
means a branch for each series, and merging them together for each
compile/test cycle. Only after doing this dozens of times did I realize
that I could stack the branches on top of each other rather than merging
each time. Then "git history fixup" and "git history reword" made it
much easier to modify the stack. But all of this has to be learned and
it is not immediately obvious.
There needs to be a compromise between friction for the contributor and
friction for the maintainer/reviewer. Unfortunately, the balance is
already so skewed in Open Source Software in general that more friction
for the individual contributor than the reviewer is currently necessary
for the maintainers/reviewers to barely avoid burning out. I
unfortunately do not have an answer other than "we know this isn't super
nice, but this is what we could come up with that makes merging your
patches more likely and faster with the resources we have today".
I also agree that it is hard to detect the actual dependencies. And in
fact they are not static: when a reviewer of one series requests
changes, it can often impact other series. Which suggests that there is
a dependency, even if it was not apparent initially.
At some point, there's also "don't send too many series at once". If
your series A requires big changes in series B, it either means it
should be series A+B, or that series B was sent too early and we should
focus on answering design questions in series A before having a look at
series B. Sometimes patience pays off in the long run.
With the issue in the previous paragraph fixed, I think this is a good
improvement and we should merge as soon as possible.
Perhaps I should just drop the whole "Of course there are sometimes
cases where a large patch series..." section?
I like the second bullet point. It also gives a way out to people: we
aren't super strict and sometimes we're fine with a series with
"different subsystems but somewhat related". I prefer the patches to
exist even if we don't review them quick than having the contributor
agonize over what it is we exactly want. We can always tell them once
their series is sent how we would have liked it to be formatted. This is
something I've learned over the years by doing, the same with learning
how to do bisectable changes, fine-grained commits and write commit logs
that are (I hope) helpful to reviewers. I believe Tom's position in
general is to leave enough wiggle room so people can figure out how
things work best for them (e.g. we don't have super strict rules for
custodians or merging, unlike the kernel for example).
Cheers,
Quentin
