patchRe: [PATCH] SubmittingPatches: address design critiques
4 messages between Jun 18, 2026 and Jun 18, 2026, from Michael Montalbo, Kristoffer Haugsbakk, Junio C Hamano.
Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.
> Contributors sometimes fail to answer fundamental design or
> viability comments from reviewers and submit subsequent rounds
> without addressing them.
I think these added sections are helpful. As a newer contributor browsing topics, I was somewhat surprised (pleasantly) how it seems like nearly every patch receives some kind of consideration even if the initial direction of a patch isn't necessarily something the project ultimately wants to adopt. I also sometimes see these kind of patches receive implementation-specific feedback in addition to design feedback, which I think can obscure the fact (especially for newcomers) that even though one continues to send re-rolls addressing implementation critique, the series won't make fundamental progress (or receive continued attention) if design issues aren't fixed first.
Show 6 quoted lines
> +You would want to be particularly mindful of critiques regarding the
> +high-level design or viability of your proposal (e.g., questioning
> +whether the feature is worth implementing, or if the chosen approach
> +is appropriate). You want to defend your design decisions on the list
> +first, because you do not want to spend too much effort in the
> +implementation if the design is not yet solid.
Two small suggestions: open with a direct imperative and replace "effort in the implementation" with "effort on the implementation".
[B]e particularly mindful of...
... too much effort [on] the implementation...> +Also, make sure that any new version is accompanied by a much clearer
> +explanation and justification (in the cover letter, your responses,
> +and in the revised commit messages). Aim to make the reviewers say
> +"it is now clear why we may want to do this with the updated version".
Maybe it would help to spell out what the explanation/justification is for more explicitly (though it may be a bit redundant with the "meaningful message" blurb):
Make sure that any new version explains and justifies those
design decisions more clearly: why the change is worth making,
what alternatives were considered, and why the chosen approach
is correct. Put that justification in the cover letter, your
responses, and the revised commit messages. Aim to make the
reviewers say "it is now clear why we may want to do this with
the updated version".On Wed, Jun 17, 2026 at 8:53 PM Michael Montalbo <mmontalbo@gmail.com> wrote:
Show 14 quoted lines
>
> Junio C Hamano wrote:
> > +You would want to be particularly mindful of critiques regarding the
> > +high-level design or viability of your proposal (e.g., questioning
> > +whether the feature is worth implementing, or if the chosen approach
> > +is appropriate). You want to defend your design decisions on the list
> > +first, because you do not want to spend too much effort in the
> > +implementation if the design is not yet solid.
>
> Two small suggestions: open with a direct imperative and replace
> "effort in the implementation" with "effort on the implementation".
>
> [B]e particularly mindful of...
> ... too much effort [on] the implementation...
Also, maybe we can even more directly say:
Do not spend too much effort on the implementation if the design is not yet solid. Be able to defend your design decisions on the list first.
On Thu, Jun 18, 2026, at 05:53, Michael Montalbo wrote:
Show 6 quoted lines
> Junio C Hamano wrote:
>>[snip]
>
> Two small suggestions: open with a direct imperative and replace
> "effort in the implementation" with "effort on the implementation".
>[snip]
The threading doesn’t work here. This is in reply to:
https://lore.kernel.org/git/xmqqv7bhxiby.fsf@gitster.g/
But your email has no `In-Reply-To` header.
Michael Montalbo <mmontalbo@gmail.com> writes:
Show 5 quoted lines
> Two small suggestions: open with a direct imperative and replace
> "effort in the implementation" with "effort on the implementation".
>
> [B]e particularly mindful of...
> ... too much effort [on] the implementation...
Yeah, I started from something like that then toned it down, but I agree that it is more appropriate to be more assertive here.
> Maybe it would help to spell out what the explanation/justification is
> for more explicitly (though it may be a bit redundant with the
> "meaningful message" blurb):
Yeah, and it depends on what kind of higher level issues the reviewer comment is about, so ...
Show 7 quoted lines
> Make sure that any new version explains and justifies those
> design decisions more clearly: why the change is worth making,
> what alternatives were considered, and why the chosen approach
> is correct. Put that justification in the cover letter, your
> responses, and the revised commit messages. Aim to make the
> reviewers say "it is now clear why we may want to do this with
> the updated version".
... I find the beginning part of the above very much better than what I had in my version, but part of the first sentence after the colon is probably a bit too much.
I'll send out v3 sometime before the next week.
Thanks.