From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Emily Shaffer <nasamuffin@google.com>
Subject: Re: [PATCH] MyFirstContribution: move preparation of v2 after review discussion
Date: Tue, 20 Oct 2026 16:33:41 +0200 [thread overview]
Message-ID: <atd7xT6Vj59omtuN@pks.im> (raw)
In-Reply-To: <xmqqa4o94e0r.fsf@gitster.g>
On Mon, Oct 19, 2026 at 01:43:48PM -0700, Junio C Hamano wrote:
> The text in the MyFirstContribution documentation tells readers how
> to send v1, how to send v2, and then how to respond to reviews, in
> this order. It is unclear how much blame can be placed on this
> text, but it somehow seems that senior community members need to
> tell new contributors not to send updated interations of their
s/interations/iterations/
> patches before having any review discussion on the current iteration
> more often these days.
>
> Here is an attempt to improve the text by moving "how to send v2"
> part much late in the text flow, after discussiong what should
> happen in the review discussion.
>
> Strengthen the language about not sending the v2 out before engaging
> in the discussion from "It is a good manners to do so" to "Do not
> send v2 before you are done".
Nit: I think this would have been easier to review if you first moved
stuff around without changing the language yet and only then changing it
in a second commit. Like this it's basically impossible to see what was
moved and what needs reviewing.
> diff --git c/Documentation/MyFirstContribution.adoc w/Documentation/MyFirstContribution.adoc
> index 34bfa2e3a9..296ff97262 100644
> --- c/Documentation/MyFirstContribution.adoc
> +++ w/Documentation/MyFirstContribution.adoc
[snip]
Now comes the rewritten part, everything above was simply moved.
> +After a few days, you will hopefully receive a reply to your patchset
> +with some comments.
> +
> +
Nit: two newlines.
> +Try to understand the reviewers' points, and ask questions if you do
> +not understand what reviewers suggested. Reviewers may also ask you
> +about what you wrote in the patchset, either in the proposed commit
> +log message or in the changes themselves. You should answer these
> +questions in your response messages, but often the reason why
> +reviewers asked these questions to understand what you meant to write
> +is because your patchset needed clarification to be understood. Make
> +a mental note that when you send an improved version of the patch,
> +make sure you explain your patch clearly enough so that the next
> +person do not have to ask the same question again.
s/do/does/
> +Try to find the best way to improve your patch they are commenting on
> +together. Until you are done discussing the current round of reviews
> +with your reviewers, do not start coding, let alone sending the next
> +version of your patch.
This may be a bit broad, as quite often I find myself exploring ideas
before I can properly respond to review feedback. But maybe it's better
to be this pushy given the recent trend of people not responding at all.
> This way both you and your reviewers will save
> +time. Reviewers can respond to your reasoning before you spend time
> +preparing a version they may not agree with, and later do not need to
> +inspect your v2 to figure out whether you implemented their comment or
> +not.
>
> If you are going to push back on a comment, be polite and explain why you feel
> your original is better; be prepared that the reviewer may still disagree with
> @@ -1501,6 +1401,115 @@ changing history, but since it's local history which you haven't shared with
> anyone, that is okay for now! (Later, it may not make sense to do this; take a
> look at the section below this one for some context.)
>
> +After review discussion concludes for the current iteration, you will
> +work on an improved version of your patch, and you will send the "v2".
Makes sense.
[snip]
> +Same as with the first version, you can use `b4 send` to send out the second
> +version. `b4` automatically bumps the version to `v2`, generates the range-diff
> +against the previous iteration, and threads the new series as a reply to the
> +cover letter of the first version.
> +
> +
> +
> +
This is a lot of empty lines here :)
Thanks!
Patrick
next prev parent reply other threads:[~2026-10-20 14:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-19 20:43 [PATCH] MyFirstContribution: move preparation of v2 after review discussion Junio C Hamano
2026-10-20 14:33 ` Patrick Steinhardt [this message]
2026-10-20 16:32 ` Junio C Hamano
2026-10-20 23:22 ` [PATCH v2 0/2] MyFirstContribution: discuss then reroll Junio C Hamano
2026-10-20 23:22 ` [PATCH v2 1/2] MyFirstContribution: move "how to send v2" after discussing reviews Junio C Hamano
2026-10-20 23:22 ` [PATCH v2 2/2] MyFirstContribution: clarify that "reviewing" is a discussion Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=atd7xT6Vj59omtuN@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=nasamuffin@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.