Re: [PATCH] SubmittingPatches: address design critiques

Re: [PATCH] SubmittingPatches: address design critiques

[Michael Montalbo]

Junio C Hamano wrote:
> 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.

> +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".

Re: [PATCH] SubmittingPatches: address design critiques

[Michael Montalbo]

On Wed, Jun 17, 2026 at 8:53 PM Michael Montalbo <mmontalbo@gmail.com> wrote:
>
> 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.

Re: [PATCH] SubmittingPatches: address design critiques

[Kristoffer Haugsbakk]

On Thu, Jun 18, 2026, at 05:53, Michael Montalbo wrote:
> 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.

Re: [PATCH] SubmittingPatches: address design critiques

[Junio C Hamano]

Michael Montalbo <mmontalbo@gmail.com> writes:

> 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 ...

>     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.

Re: [PATCH] SubmittingPatches: address design critiques

[Michael Montalbo]

On Wed, Jun 17, 2026 at 8:53 PM Michael Montalbo <mmontalbo@gmail.com> wrote:
>
> 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...

(resending to fix threading)

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.

[PATCH] SubmittingPatches: address design critiques

[Junio C Hamano]

Contributors sometimes fail to answer fundamental design or
viability comments from reviewers and submit subsequent rounds
without addressing them.  When design decisions are resolved on the
mailing list, the final justification should be recorded in the
commit messages.

Instruct authors to be particularly mindful of critiques regarding
high-level design or viability, to defend their choices on the list,
and to accompany new iterations with clearer explanations in the cover
letter, responses, and revised commit messages. Also instruct them to
explicitly document the resolution of these concerns in the commit
message body to keep the historical record complete.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/SubmittingPatches | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 176567738d..bfe3745a54 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -51,6 +51,21 @@ area.
   respond to them with "Reply-All" on the mailing list, while taking
   them into account while preparing an updated set of patches.
 +
+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.
++
+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".
++
+Topics that fail to address fundamental design critiques without
+resolution will not be considered ready for merging.
++
 It is often beneficial to allow some time for reviewers to provide
 feedback before sending a new version, rather than sending an updated
 series immediately after receiving a review. This helps collect broader
@@ -322,6 +337,10 @@ The body should provide a meaningful commit message, which:
 
 . alternate solutions considered but discarded, if any.
 
+. the resolution of design or viability concerns raised by the
+  community during the review, if any, ensuring the historical record
+  explains why the chosen approach was accepted over alternatives.
+
 [[present-tense]]
 The problem statement that describes the status quo is written in the
 present tense.  Write "The code does X when it is given input Y",
-- 
2.55.0-rc1-92-ge545aa9d3e