Volume XXII, number 280Wednesday, October 7, 2026Latest message 2 hours ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patchSubmittingPatches: address design critiques

8 messages between Jun 17, 2026 and Jun 22, 2026, from Junio C Hamano, Kristoffer Haugsbakk, Patrick Steinhardt.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Junio C HamanoJun 17, 2026, 16:06 UTC on lore

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(+)
Show changes to Documentation/SubmittingPatches +19 −0
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
Junio C HamanoJun 18, 2026, 08:50 UTC in reply to Junio C Hamano on lore

[PATCH v2] SubmittingPatches: address design critiques

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>
---
 * Rephrased the instruction in the first hunk somewhat to be a bit
   more explicit, and added a missing verb to the second hunk.
 Documentation/SubmittingPatches | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
Show changes to Documentation/SubmittingPatches +19 −0
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f042bb5aaf..bbe759f3d9 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 should 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).  Defend your design decisions on the list first, to
+avoid wasting effort on an implementation whose design is not yet
+solid.
++
+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
@@ -323,6 +338,10 @@ The body should provide a meaningful commit message, which:
 
 . alternate solutions considered but discarded, if any.
 
+. records 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",

Range-diff against v1:
1:  eb5f96ab04 ! 1:  aecdcf0bda SubmittingPatches: address design critiques
    @@ Documentation/SubmittingPatches: 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
    ++You should 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.
    ++is appropriate).  Defend your design decisions on the list first, to
    ++avoid wasting effort on an implementation whose design is not yet
    ++solid.
     ++
    -+Also, make sure that any new version is accompanied by a much clearer
    ++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".
    @@ Documentation/SubmittingPatches: The body should provide a meaningful commit mes
      
      . alternate solutions considered but discarded, if any.
      
    -+. the resolution of design or viability concerns raised by the
    ++. records 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.
     +

-- 
2.55.0-rc1-93-ge727df1850
Kristoffer HaugsbakkJun 18, 2026, 14:43 UTC in reply to Junio C Hamano on lore

Re: [PATCH v2] SubmittingPatches: address design critiques

On Thu, Jun 18, 2026, at 10:50, Junio C Hamano wrote:
Show 5 quoted lines
> 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.

It’s useful to explain design decisions etc. that were not committed to in the commit message. It’s yet another thing that might be far from obvious during review, but when the message is written only the option that was landed on is explained.

Show 15 quoted lines
>[snip]
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index f042bb5aaf..bbe759f3d9 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 should 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).  Defend your design decisions on the list first, to
> +avoid wasting effort on an implementation whose design is not yet
> +solid.

To take it to the other extreme, there are “contributions” which are design decisions *only*, no implementation. Namely emails which sketch a design like a new option. Those are often met with a stock response saying:[1] submit some code, doesn’t have to implement it fully, but we will need some code to proceed here. I’ve never understood that stance, and I don’t think it harmonizes with the sentence here of sparing effort if the design is not solid.

Many of these emails are straightforward.
1. I want this thing
2. I can implement it but I don’t know if *would be* accepted (and
   implementing things here would be a lot of effort for me, personally)
3. Would it be?

And the only thing I personally would change is to weaken “would be?” to “could be?”. Then with that out in the world, maybe someone finds this message this day or the next week or month and reports that at least they would like this thing.

Some people could change something small in this code base as easily as they could propose the change idea for it (so it seems to me). They would just have to take a walk and watch a movie to let their subconscious process whether it was a good idea or not... But for most people, committing to implementing something in a new codebase without spooky assistance (...) is a task that takes a lot of effort.

To illustrate a related point, there are at least three kinds of Git users out there:

1. Experienced C developers
2. Experienced Git developers (heavy overlap with (1))
3. Experienced and partisan Git users (knows it, believes in it)

You can imagine someone from group number 1 who is *not* in group number 3 use a weekend to implement something. But then when it is submitted it turns out that is a very “centralized CVS” idea which doesn’t fit into git(1) at all. That’s easily spotted by group number 3 by just looking at the proposed docs or design. Now that group number 1 individual might just have a bunch of code that is dead weight for any proper Git workflow.

So I don’t understand this canned response.

There is the unofficial project idea tracker on the GitGitGadget fork. But it seems weird to post things there and not on the mailing list.

† 1: From `git show todo:CannedResponses`:
     | [make us come to you, begging]
     |
     | I've seen from time to time people ask "I am thinking of doing this;
     | will a patch be accepted?  If so, I'll work on it." before showing
     | any work, and my response always has been:
     |
     |  (1) We don't know how useful and interesting your contribution would
     |      be for our audience, until we see it; and
     |
     |  (2) If you truly believe in your work (find it useful, find writing
     |      it fun, etc.), that would be incentive enough for you to work
     |      on it, whether or not the result will land in my tree.  You
     |      should instead aim for something so brilliant that we would
     |      come to you begging for your permission to include it in our
     |      project.
    Note that point (2) is a bit more ambitious than it looks; if the
    idea is to implement your own thing for your own fork/tree and then
    maintain it yourself if upstream git.git doesn not accept it, well,
    now you need to learn how to maintain a fork for however long you
    want that feature. If you did not already know that.
Show 8 quoted lines
> ++
> +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.
Nicely explained.
Show 12 quoted lines
> ++
>  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
> @@ -323,6 +338,10 @@ The body should provide a meaningful commit message, which:
>
>  . alternate solutions considered but discarded, if any.
>
> +. records 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.
> +
Okay.
>  [[present-tense]]
>[snip]
Junio C HamanoJun 18, 2026, 16:26 UTC in reply to Kristoffer Haugsbakk on lore

Re: [PATCH v2] SubmittingPatches: address design critiques

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
Show 7 quoted lines
> You can imagine someone from group number 1 who is *not* in group number
> 3 use a weekend to implement something. But then when it is submitted it
> turns out that is a very “centralized CVS” idea which doesn’t fit into
> git(1) at all. That’s easily spotted by group number 3 by just looking
> at the proposed docs or design. Now that group number 1 individual might
> just have a bunch of code that is dead weight for any proper Git
> workflow.

That depends on how obviously wrong the idea is. If your proposal is to write another CVS into Git, that may be too obvious it may not fly, but the thing is, "proposals" that get the canned response you quoted are often vague enough that crucial details that divide "iffy" and "obviously wrong" are missing.

One way to make these proposals sufficiently clear to allow reviewers to tell the difference is with a code that builds. There may be other ways, but that is one obvious way to start a meaningful discussion.

Junio C HamanoJun 19, 2026, 16:17 UTC in reply to Junio C Hamano on lore

[PATCH v3] SubmittingPatches: address design critiques

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>
---
 * Updated a bit after reading comments by Kichael and Kristoffer
 Documentation/SubmittingPatches | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
Show changes to Documentation/SubmittingPatches +19 −0
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f042bb5aaf..a9789e5303 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.
 +
+Be particularly mindful of critiques regarding the high-level design
+or viability of your proposal (e.g., questioning if the feature is
+worth implementing, or if the chosen approach is appropriate).  Defend
+your design decisions on the list first, work with reviewers and other
+members to improve the design before revising the implementation, to
+avoid wasting effort on an implementation before its design is solid.
++
+Make sure that any new version explains and justifies those design
+decisions more clearly, in the cover letter 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 with unresolved fundamental design critiques 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
@@ -323,6 +338,10 @@ The body should provide a meaningful commit message, which:
 
 . alternate solutions considered but discarded, if any.
 
+. records 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-129-gff98d784de
Junio C HamanoJun 20, 2026, 23:43 UTC in reply to Junio C Hamano on lore

[PATCH v4] SubmittingPatches: address design critiques

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>
---
 * Hopefully this will be the last iteration.
 Documentation/SubmittingPatches | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
Show changes to Documentation/SubmittingPatches +20 −0
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f042bb5aaf..28b4f2f795 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -51,6 +51,22 @@ area.
   respond to them with "Reply-All" on the mailing list, while taking
   them into account while preparing an updated set of patches.
 +
+Be particularly mindful of critiques regarding the high-level design
+or viability of your proposal (e.g., questioning if the feature is
+worth implementing, or if the chosen approach is appropriate).  Defend
+your design decisions on the list first and work with reviewers and
+other members to improve the design before revising the implementation.
+This will avoid wasting effort on an implementation before its design is
+solid.
++
+Make sure that any new version explains and justifies those design
+decisions more clearly, in the cover letter 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 with unresolved fundamental design critiques 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
@@ -323,6 +339,10 @@ The body should provide a meaningful commit message, which:
 
 . alternate solutions considered but discarded, if any.
 
+. records 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",

Interdiff against v2:
  diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
  index a9789e5303..28b4f2f795 100644
  --- a/Documentation/SubmittingPatches
  +++ b/Documentation/SubmittingPatches
  @@ -54,9 +54,10 @@ area.
   Be particularly mindful of critiques regarding the high-level design
   or viability of your proposal (e.g., questioning if the feature is
   worth implementing, or if the chosen approach is appropriate).  Defend
  -your design decisions on the list first, work with reviewers and other
  -members to improve the design before revising the implementation, to
  -avoid wasting effort on an implementation before its design is solid.
  +your design decisions on the list first and work with reviewers and
  +other members to improve the design before revising the implementation.
  +This will avoid wasting effort on an implementation before its design is
  +solid.
   +
   Make sure that any new version explains and justifies those design
   decisions more clearly, in the cover letter and in the revised commit

-- 
2.55.0-rc1-134-gb7e3543e07
Patrick SteinhardtJun 22, 2026, 08:20 UTC in reply to Junio C Hamano on lore

Re: [PATCH v4] SubmittingPatches: address design critiques

On Sat, Jun 20, 2026 at 04:43:00PM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> 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>
> ---
>  * Hopefully this will be the last iteration.
This version looks good to me, thanks!
Patrick
Junio C HamanoJun 22, 2026, 12:57 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH v4] SubmittingPatches: address design critiques

Patrick Steinhardt <ps@pks.im> writes:
Show 21 quoted lines
> On Sat, Jun 20, 2026 at 04:43:00PM -0700, Junio C Hamano wrote:
>> 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>
>> ---
>>  * Hopefully this will be the last iteration.
>
> This version looks good to me, thanks!
>
> Patrick
Thanks.

Back to recent threads