[PATCH] MyFirstContribution: refrain from self-iterating too much

[PATCH] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Finding mistakes in and improving your own patches is a good idea,
but doing so too quickly is being inconsiderate to reviewers who
have just seen the initial iteration and taking their time to review
it.  Encourage new developers to perform such a self review before
they send out their patches, not after.

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

diff --git c/Documentation/MyFirstContribution.txt w/Documentation/MyFirstContribution.txt
index 7c9a037cc2..81dcaedf0c 100644
--- c/Documentation/MyFirstContribution.txt
+++ w/Documentation/MyFirstContribution.txt
@@ -1136,6 +1136,26 @@ index 88f126184c..38da593a60 100644
 [[now-what]]
 == My Patch Got Emailed - Now What?
 
+You should wait for your patch to be reviewed by other people in the
+development community.  While you are waiting, you may want to
+re-read what you wrote in the patch you already have sent, as if you
+are a reviewer who is helping you to improve your patch.  But resist
+the temptation to update the patch and send a new one, until other
+people had enough time to digest your original patch and give you
+their reviews.  They may be taking time to give you a carefully
+written review responses and haven't finished it yet.  Bombarding
+them with new versions before they have a chance to react to the
+first iteration is being rude to them.
+
+Of course, you still may spot mistakes and rooms for improvements
+after you sent your initial patch.  Learn from that experience to
+make sure that you will do such a self-review _before_ sending your
+patches next time.  You do not have to send your patches immediately
+once you finished writing them.  It is not a race.  Take your time
+to self-review them, sleep on them, improve them before sending them
+out, and do not allow you to send them before you are reasonably
+sure that you won't find more mistakes in them yourself.
+
 [[reviewing]]
 === Responding to Reviews
 

Re: [PATCH] MyFirstContribution: refrain from self-iterating too much

[Torsten Bögershausen]

On Sat, Jan 21, 2023 at 05:51:11PM -0800, Junio C Hamano wrote:
> Finding mistakes in and improving your own patches is a good idea,
> but doing so too quickly is being inconsiderate to reviewers who
> have just seen the initial iteration and taking their time to review
> it.  Encourage new developers to perform such a self review before
> they send out their patches, not after.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/MyFirstContribution.txt | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git c/Documentation/MyFirstContribution.txt w/Documentation/MyFirstContribution.txt
> index 7c9a037cc2..81dcaedf0c 100644
> --- c/Documentation/MyFirstContribution.txt
> +++ w/Documentation/MyFirstContribution.txt
> @@ -1136,6 +1136,26 @@ index 88f126184c..38da593a60 100644
>  [[now-what]]
>  == My Patch Got Emailed - Now What?
>
> +You should wait for your patch to be reviewed by other people in the
> +development community.  While you are waiting, you may want to
> +re-read what you wrote in the patch you already have sent, as if you
> +are a reviewer who is helping you to improve your patch.  But resist
> +the temptation to update the patch and send a new one, until other
> +people had enough time to digest your original patch and give you
> +their reviews.  They may be taking time to give you a carefully
> +written review responses and haven't finished it yet.  Bombarding
> +them with new versions before they have a chance to react to the
> +first iteration is being rude to them.
> +
> +Of course, you still may spot mistakes and rooms for improvements
> +after you sent your initial patch.

I can't resist the question: After outlining what not to do and why,
could there be a hint what to do ?
It may be, that the author justs spots a simple typo, or there may
be more heavier changes to be done.

Should the author just respond to her/his patch as a reviewer does ?
Like:
Ops, there is a "typax", I should have written "typo".
Or:
Re-reading my own stuff, I think that things could have been done
in a way like this....
Lets wait for more comments before I send out a new version.

>+ Learn from that experience to
> +make sure that you will do such a self-review _before_ sending your
> +patches next time.  You do not have to send your patches immediately
> +once you finished writing them.  It is not a race.  Take your time
> +to self-review them, sleep on them, improve them before sending them
> +out, and do not allow you to send them before you are reasonably
> +sure that you won't find more mistakes in them yourself.
> +
>  [[reviewing]]
>  === Responding to Reviews
>

Re: [PATCH] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

>> +Of course, you still may spot mistakes and rooms for improvements
>> +after you sent your initial patch.
>
> I can't resist the question: After outlining what not to do and why,
> could there be a hint what to do ?

There already is and it is the theme of the next paragraph.

People on the list do not have to see your patch immediately after
you wrote it.  Instead of seeing the initial version right now that
is followed by a series of "oops, I like this verison better than
the previous one" rerolls over 2 days, reviewers would appreciate if
a single more polished version came 2 days late and that version was
the only one they need to review.

Wait, re-read what you wrote, fix the problems you find locally, all
without sending it out until you find no more "oops, that would not
work" and simple typos.  Sleep on it.  

Of course, people are not perfect so they may still find issues
after they sent their patches out.

> It may be, that the author justs spots a simple typo, or there may
> be more heavier changes to be done.
>
> Should the author just respond to her/his patch as a reviewer does ?
> Like:
> Ops, there is a "typax", I should have written "typo".

Follow that with the same "I will fix this typo when I reroll, but
I'll wait for reviews from others" as the other one, and it would be
the second best thing you could do (the best is to avoid having to
say that, of course).

> Or:
> Re-reading my own stuff, I think that things could have been done
> in a way like this....
> Lets wait for more comments before I send out a new version.

Again, very good.

Re: [PATCH] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Torsten Bögershausen <tboegi@web.de> writes:
>
>> Or:
>> Re-reading my own stuff, I think that things could have been done
>> in a way like this....
>> Lets wait for more comments before I send out a new version.
>
> Again, very good.

The latter one (different course) needs spearate treatment from the
former (typofix).  Small corrections without changing course is
better communicated with "will fix when I reroll" while a drastic
change should warn the readers not to waste their time on the
premature initial version, i.e. "I came up with a drastically
different and better idea; please ignore the one I just posted and
wait for an updated one".

I'll sleep on what I rewrote to see if I or other people find better
phrasing.

Thanks.

[PATCH v2] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Finding mistakes in and improving your own patches is a good idea,
but doing so too quickly is being inconsiderate to reviewers who
have just seen the initial iteration and taking their time to review
it.  Encourage new developers to perform such a self review before
they send out their patches, not after.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/MyFirstContribution.txt | 30 +++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index ccfd0cb5f3..3e4f1c7764 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -1256,6 +1256,36 @@ index 88f126184c..38da593a60 100644
 [[now-what]]
 == My Patch Got Emailed - Now What?
 
+After you sent out your first patch, you may find mistakes in it, or
+a different and better way to achieve the goal of the patch.  But
+resist the temptation to send a new version immediately.
+
+ - If the mistakes you found are minor, send a reply to your patch as
+   if you were a reviewer and mention that you will fix them in an
+   updated version.
+
+ - On the other hand, if you think you want to change the course so
+   drastically that reviews on the initial patch would become
+   useless, send a reply to your patch to say so immediately to
+   avoid wasting others' time (e.g. "I am working on a better
+   approach, so please ignore this patch, and wait for the updated
+   version.")
+
+And give reviewers enough time to process your initial patch before
+sending an updated version.
+
+The above is a good practice if you sent your initial patch
+prematurely without polish.  But a better approach of course is to
+avoid sending your patch prematurely in the first place.
+
+Keep in mind that people in the development community do not have to
+see your patch immediately after you wrote it.  Instead of seeing
+the initial version right now, that you will follow up with several
+updated "oops, I like this version better than the previous one"
+versions over 2 days, reviewers would much appreciate if a single
+more polished version came 2 days late and that version, that
+contains fewer mistakes, were the only one they need to review.
+
 [[reviewing]]
 === Responding to Reviews
 
-- 
2.39.1-308-g56c8fb1e95

Re: [PATCH v2] MyFirstContribution: refrain from self-iterating too much

[Torsten Bögershausen]

On Sun, Jan 22, 2023 at 08:18:05PM -0800, Junio C Hamano wrote:

The whole thing is much more convenient to read, so to say.
Some nit-picking inline.

> Finding mistakes in and improving your own patches is a good idea,
> but doing so too quickly is being inconsiderate to reviewers who
> have just seen the initial iteration and taking their time to review
> it.  Encourage new developers to perform such a self review before
> they send out their patches, not after.

I think that this is what V1 was about. Review first, send then.
Is there still so much focus on this ?
Or is it more about "what to do when it went wrong?"

How about this, or similar ?

...it.  Encourage developers to wait with a new version too early.
But if they plan to send one later, they are welcome to comment
their own work as they where reviers.


>
> Helped-by: Torsten Bögershausen <tboegi@web.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/MyFirstContribution.txt | 30 +++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
> index ccfd0cb5f3..3e4f1c7764 100644
> --- a/Documentation/MyFirstContribution.txt
> +++ b/Documentation/MyFirstContribution.txt
> @@ -1256,6 +1256,36 @@ index 88f126184c..38da593a60 100644
>  [[now-what]]
>  == My Patch Got Emailed - Now What?
>
> +After you sent out your first patch, you may find mistakes in it, or
> +a different and better way to achieve the goal of the patch.  But
> +resist the temptation to send a new version immediately.
> +
> + - If the mistakes you found are minor, send a reply to your patch as
> +   if you were a reviewer and mention that you will fix them in an
> +   updated version.
> +
> + - On the other hand, if you think you want to change the course so
> +   drastically that reviews on the initial patch would become
> +   useless, send a reply to your patch to say so immediately to
> +   avoid wasting others' time (e.g. "I am working on a better
> +   approach, so please ignore this patch, and wait for the updated
> +   version.")
> +
> +And give reviewers enough time to process your initial patch before
> +sending an updated version.
> +
> +The above is a good practice if you sent your initial patch
> +prematurely without polish.  But a better approach of course is to
> +avoid sending your patch prematurely in the first place.
> +
> +Keep in mind that people in the development community do not have to
> +see your patch immediately after you wrote it.  Instead of seeing
> +the initial version right now, that you will follow up with several
> +updated "oops, I like this version better than the previous one"
> +versions over 2 days, reviewers would much appreciate if a single
> +more polished version came 2 days late and that version, that
> +contains fewer mistakes, were the only one they need to review.
> +
>  [[reviewing]]
>  === Responding to Reviews
>
> --
> 2.39.1-308-g56c8fb1e95
>

[PATCH v3] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Finding mistakes in and improving your own patches is a good idea,
but doing so too quickly is being inconsiderate to reviewers who
have just seen the initial iteration and taking their time to review
it.  Encourage new developers to perform such a self review before
they send out their patches, not after.  After sending a patch that
they immediately found mistakes in, they are welcome to comment on
them, mentioning what and how they plan to improve them in an
updated version, before sending out their updates.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Sorry for a slow update.  Even though the topic is about not
   updating too quickly, this update was long overdue.  Not a whole
   lot changed.  Primary change is the later part of the proposed
   log message, which was helped by Torsten's comments, to which
   this message is a response to.

 Documentation/MyFirstContribution.txt | 31 +++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index ccfd0cb5f3..1ede3f8e37 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -1256,6 +1256,37 @@ index 88f126184c..38da593a60 100644
 [[now-what]]
 == My Patch Got Emailed - Now What?
 
+After you sent out your first patch, you may find mistakes in it, or
+a different and better way to achieve the goal of the patch.  But
+please resist the temptation to send a new version immediately.
+
+ - If the mistakes you found are minor, send a reply to your patch as
+   if you were a reviewer and mention that you will fix them in an
+   updated version.
+
+ - On the other hand, if you think you want to change the course so
+   drastically that reviews on the initial patch would become
+   useless, send a reply to your patch to say so immediately to
+   avoid wasting others' time (e.g. "I am working on a better
+   approach, so please ignore this patch, and wait for the updated
+   version.")
+
+Then give reviewers enough time to process your initial patch before
+sending an updated version (unless you retracted the initial patch,
+that is).
+
+Now, the above is a good practice if you sent your initial patch
+prematurely without polish.  But a better approach of course is to
+avoid sending your patch prematurely in the first place.
+
+Keep in mind that people in the development community do not have to
+see your patch immediately after you wrote it.  Instead of seeing
+the initial version right now, that will be followed by several
+updated "oops, I like this version better than the previous one"
+versions over 2 days, reviewers would more appreciate if a single
+polished version came 2 days late and that version with fewer
+mistakes were the only one they need to review.
+
 [[reviewing]]
 === Responding to Reviews
 
-- 
2.41.0-376-gcba07a324d

Re: [PATCH v3] MyFirstContribution: refrain from self-iterating too much

[Linus Arver]

I was in the process of writing a review but my comments were getting a
bit long. So to save you the trouble of applying the suggested changes,
I have a scissors patch version at the bottom with the changes applied
(you may simply want to read that first).

Junio C Hamano <gitster@pobox.com> writes:

> +After you sent out your first patch, you may find mistakes in it, or

s/you sent/sending

Also, add "perhaps realize" after "or".

> +a different and better way to achieve the goal of the patch.  But
> +please resist the temptation to send a new version immediately.
> +
> + - If the mistakes you found are minor, send a reply to your patch as
> +   if you were a reviewer and mention that you will fix them in an

s/a/the

> +   updated version.
> +
> + - On the other hand, if you think you want to change the course so
> +   drastically that reviews on the initial patch would become
> +   useless, send a reply to your patch to say so immediately to
> +   avoid wasting others' time (e.g. "I am working on a better
> +   approach, so please ignore this patch, and wait for the updated
> +   version.")

How about this wording?

     - On the other hand, if you think you want to change the course so
       drastically that reviews on the initial patch would be a waste of
       time (for everyone involved), retract the patch immediately with
       a reply like "I am working on a much better approach, so please
       ignore this patch and wait for the updated version."
 
> +Then give reviewers enough time to process your initial patch before
> +sending an updated version (unless you retracted the initial patch,
> +that is).

I think this paragraph should be moved inside the first one above. So it
could read something like this:

    Please give reviewers enough time to process your initial patch before
    sending an updated version. That is, resist the temptation to send a new
    version immediately (because it may be the case that others may have
    already started reviewing your initial version).

    While waiting for review comments, you may find mistakes in your initial
    patch, or perhaps realize a different and better way to achieve the goal
    of the patch. In this case you may communicate your findings to other
    reviewers as follows:

> +Now, the above is a good practice if you sent your initial patch
> +prematurely without polish.  But a better approach of course is to
> +avoid sending your patch prematurely in the first place.

OK.

> +Keep in mind that people in the development community do not have to
> +see your patch immediately after you wrote it.

How about just

     Please be considerate of the time needed by reviewers to examine
     each new version of your patch.

?

> Instead of seeing

s/Instead of/Rather than

> +the initial version right now, that will be followed by several

s/, that will be / (

> +updated "oops, I like this version better than the previous one"

s/updated//

> +versions over 2 days, reviewers would more appreciate if a single

s/versions over 2 days,/patches over 2 days),

s/more appreciate/strongly prefer

> +polished version came 2 days late and that version with fewer

s/2 days late/instead,

> +mistakes were the only one they need to review.

s/need/would need/
> +
>  [[reviewing]]
>  === Responding to Reviews
>  
> -- 
> 2.41.0-376-gcba07a324d

Below is my scissor patch version.

--8<---------------cut here---------------start------------->8---
Please give reviewers enough time to process your initial patch before
sending an updated version. That is, resist the temptation to send a new
version immediately (because it may be the case that others may have
already started reviewing your initial version).

While waiting for review comments, you may find mistakes in your initial
patch, or perhaps realize a different and better way to achieve the goal
of the patch. In this case you may communicate your findings to other
reviewers as follows:

 - If the mistakes you found are minor, send a reply to your patch as if
   you were the reviewer and mention that you will fix them in an
   updated version.

 - On the other hand, if you think you want to change the course so
   drastically that reviews on the initial patch would be a waste of
   time (for everyone involved), retract the patch immediately with
   a reply like "I am working on a much better approach, so please
   ignore this patch and wait for the updated version."

Now, the above is a good practice if you sent your initial patch
prematurely without polish.  But a better approach of course is to avoid
sending your patch prematurely in the first place.

Please be considerate of the time needed by reviewers to examine each
new version of your patch. Rather than seeing the initial version right
now (followed by several "oops, I like this version better than the
previous one" patches over 2 days), reviewers would strongly prefer if a
single polished version came instead, and that version with fewer
mistakes were the only one they would need to review.
--8<---------------cut here---------------end--------------->8---
 

Re: [PATCH v3] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Linus Arver <linusa@google.com> writes:

> I was in the process of writing a review but my comments were getting a
> bit long. So to save you the trouble of applying the suggested changes,
> I have a scissors patch version at the bottom with the changes applied
> (you may simply want to read that first).

Thanks.  I very much like your version over the one you are
responding to, except for one minor point.

>> +Keep in mind that people in the development community do not have to
>> +see your patch immediately after you wrote it.
>
> How about just
>
>      Please be considerate of the time needed by reviewers to examine
>      each new version of your patch.
>
> ?

Both give a useful piece of advice, but they are slightly different.

> Please be considerate of the time needed by reviewers to examine each
> new version of your patch. Rather than seeing the initial version right
> now (followed by several "oops, I like this version better than the
> previous one" patches over 2 days), reviewers would strongly prefer if a
> single polished version came instead, and that version with fewer
> mistakes were the only one they would need to review.

What I wanted to convey with "we do not need your initial patch
immediately" was that it is perfectly fine if the cost of producing
the version "reviewers would strongly prefer" is that it takes 2
more days before they see such a more polished version.  IOW, adding
something like
 
   , even if it took 2 more days before they see the version.

at the end of the above paragraph was what I wanted to say with
"hey, this is not a race. don't focus on sending immediately after
you wrote it. nobody is dying to see your patch immediately off the
press".

Thanks.

[PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Finding mistakes in and improving your own patches is a good idea,
but doing so too quickly is being inconsiderate to reviewers who
have just seen the initial iteration and taking their time to review
it.  Encourage new developers to perform such a self review before
they send out their patches, not after.  After sending a patch that
they immediately found mistakes in, they are welcome to comment on
them, mentioning what and how they plan to improve them in an
updated version, before sending out their updates.

Helped-by: Torsten Bögershausen <tboegi@web.de>
Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

The inter/range-diff with my v3 was totally useless, but here is to
show three minor edits I made to Linus's version I am responding to.

 * Simplify parenthesized "because it may be the case that".

 * As if you were "a" reviewer, as we do not designate "the
   reviewer(s)" to a patch.  Anybody can (volunteer to) be a
   reviewer for a patch, and you can be, too.

 * Stress that a single polished patch that comes later (because it
   took time to polish) is vastly preferred than flurry of "oops
   this is better" updates.

    --- /var/tmp/b	2023-07-27 17:38:56.928040307 -0700
    +++ /var/tmp/a	2023-07-27 17:38:36.100067020 -0700
    @@ -1,7 +1,7 @@
     Please give reviewers enough time to process your initial patch before
     sending an updated version. That is, resist the temptation to send a new
    -version immediately (because it may be the case that others may have
    -already started reviewing your initial version).
    +version immediately, because others may have already started reviewing
    +your initial version.
     
     While waiting for review comments, you may find mistakes in your initial
     patch, or perhaps realize a different and better way to achieve the goal
    @@ -9,7 +9,7 @@
     reviewers as follows:
     
      - If the mistakes you found are minor, send a reply to your patch as if
    -   you were the reviewer and mention that you will fix them in an
    +   you were a reviewer and mention that you will fix them in an
        updated version.
     
      - On the other hand, if you think you want to change the course so
    @@ -26,5 +26,5 @@
     new version of your patch. Rather than seeing the initial version right
     now (followed by several "oops, I like this version better than the
     previous one" patches over 2 days), reviewers would strongly prefer if a
    -single polished version came instead, and that version with fewer
    -mistakes were the only one they would need to review.
    +single polished version came 2 days later instead, and that version with
    +fewer mistakes were the only one they would need to review.

Thanks.


 Documentation/MyFirstContribution.txt | 32 +++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/Documentation/MyFirstContribution.txt b/Documentation/MyFirstContribution.txt
index ccfd0cb5f3..dd46f751b7 100644
--- a/Documentation/MyFirstContribution.txt
+++ b/Documentation/MyFirstContribution.txt
@@ -1256,6 +1256,38 @@ index 88f126184c..38da593a60 100644
 [[now-what]]
 == My Patch Got Emailed - Now What?
 
+Please give reviewers enough time to process your initial patch before
+sending an updated version. That is, resist the temptation to send a new
+version immediately, because others may have already started reviewing
+your initial version.
+
+While waiting for review comments, you may find mistakes in your initial
+patch, or perhaps realize a different and better way to achieve the goal
+of the patch. In this case you may communicate your findings to other
+reviewers as follows:
+
+ - If the mistakes you found are minor, send a reply to your patch as if
+   you were a reviewer and mention that you will fix them in an
+   updated version.
+
+ - On the other hand, if you think you want to change the course so
+   drastically that reviews on the initial patch would be a waste of
+   time (for everyone involved), retract the patch immediately with
+   a reply like "I am working on a much better approach, so please
+   ignore this patch and wait for the updated version."
+
+Now, the above is a good practice if you sent your initial patch
+prematurely without polish.  But a better approach of course is to avoid
+sending your patch prematurely in the first place.
+
+Please be considerate of the time needed by reviewers to examine each
+new version of your patch. Rather than seeing the initial version right
+now (followed by several "oops, I like this version better than the
+previous one" patches over 2 days), reviewers would strongly prefer if a
+single polished version came 2 days later instead, and that version with
+fewer mistakes were the only one they would need to review.
+
+
 [[reviewing]]
 === Responding to Reviews
 
-- 
2.41.0-470-gbfce02c22f

Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Jacob Abel]

On 23/07/27 05:43PM, Junio C Hamano wrote:
> Finding mistakes in and improving your own patches is a good idea,
> but doing so too quickly is being inconsiderate to reviewers who
> have just seen the initial iteration and taking their time to review
> it.  Encourage new developers to perform such a self review before
> they send out their patches, not after.  After sending a patch that
> they immediately found mistakes in, they are welcome to comment on
> them, mentioning what and how they plan to improve them in an
> updated version, before sending out their updates.
> 
> [...]
>
> +Please be considerate of the time needed by reviewers to examine each
> +new version of your patch. Rather than seeing the initial version right
> +now (followed by several "oops, I like this version better than the
> +previous one" patches over 2 days), reviewers would strongly prefer if a
> +single polished version came 2 days later instead, and that version with
> +fewer mistakes were the only one they would need to review.
> +
> +
> [...]

Speaking as a fairly green contributor to the project, it may be helpful
to include some guidance on what is "too long" vs "too short" when
waiting to send out the next revision. 

Likewise it may be worthwhile to mention how the expected "minimum time
between revisions" will generally shrink as you get to higher revision
counts and fewer changes between revisions.

Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Jacob Abel <jacobabel@nullpo.dev> writes:

> On 23/07/27 05:43PM, Junio C Hamano wrote:
>> Finding mistakes in and improving your own patches is a good idea,
>> but doing so too quickly is being inconsiderate to reviewers who
>> have just seen the initial iteration and taking their time to review
>> it.  Encourage new developers to perform such a self review before
>> they send out their patches, not after.  After sending a patch that
>> they immediately found mistakes in, they are welcome to comment on
>> them, mentioning what and how they plan to improve them in an
>> updated version, before sending out their updates.
>> 
>> [...]
>>
>> +Please be considerate of the time needed by reviewers to examine each
>> +new version of your patch. Rather than seeing the initial version right
>> +now (followed by several "oops, I like this version better than the
>> +previous one" patches over 2 days), reviewers would strongly prefer if a
>> +single polished version came 2 days later instead, and that version with
>> +fewer mistakes were the only one they would need to review.
>> +
>> +
>> [...]
>
> Speaking as a fairly green contributor to the project, it may be helpful
> to include some guidance on what is "too long" vs "too short" when
> waiting to send out the next revision. 

We generally do not want to be too prescriptive, as the right
interval depends on many factors like the complexity of the topic,
how busy the reviewers are otherwise, etc.  And that is why I did
not go any more specific than "several rounds within 2 days is way
too frequent".

But as a general common-sense guideline, I would encourage people to
wait for at least one earth rotation, given that there are list
participants across many timezones.  I do not know offhand how to
fit that well in the narrative being proposed, though.

> Likewise it may be worthwhile to mention how the expected "minimum time
> between revisions" will generally shrink as you get to higher revision
> counts and fewer changes between revisions.

I am not sure if I follow.  As a topic gets iterated and getting
closer to completion, maximum time allowed between revisions to keep
the minds of those involved in the topic fresh may shrink, but I do
not think it would affect the minimum interval too much.

Thanks.

Re* [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
>> Speaking as a fairly green contributor to the project, it may be helpful
>> to include some guidance on what is "too long" vs "too short" when
>> waiting to send out the next revision. 
>
> We generally do not want to be too prescriptive, ...
> But as a general common-sense guideline, I would encourage people to
> wait for at least one earth rotation, given that there are list
> participants across many timezones.  I do not know offhand how to
> fit that well in the narrative being proposed, though.

On top of v4, we could do something like this, I guess, but I
realize that this is talking about minimum waiting time to allow
others to even notice-see your patches, while the original is about
them needing time after noticing your patches to process them, and
the latter heavily depend on many factors (like how involved the
patches are, how many people are likely to be interested in).

So, I doubt adding this is a good idea.

diff --git c/Documentation/MyFirstContribution.txt w/Documentation/MyFirstContribution.txt
index 93c9e459fc..440e9ede32 100644
--- c/Documentation/MyFirstContribution.txt
+++ w/Documentation/MyFirstContribution.txt
@@ -1259,7 +1259,9 @@ index 88f126184c..38da593a60 100644
 Please give reviewers enough time to process your initial patch before
 sending an updated version. That is, resist the temptation to send a new
 version immediately, because others may have already started reviewing
-your initial version.
+your initial version. The development community members are across the
+globe and it is a good idea to give them time to see your patches by
+waiting for at least one rotation of the earth.
 
 While waiting for review comments, you may find mistakes in your initial
 patch, or perhaps realize a different and better way to achieve the goal

Re: Re* [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Jacob Abel]

On 23/07/28 08:42AM, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Jacob Abel <jacobabel@nullpo.dev> writes:
> >
> > [...]
> 
> On top of v4, we could do something like this, I guess, but I
> realize that this is talking about minimum waiting time to allow
> others to even notice-see your patches, while the original is about
> them needing time after noticing your patches to process them, and
> the latter heavily depend on many factors (like how involved the
> patches are, how many people are likely to be interested in).
> 
> So, I doubt adding this is a good idea.
>
> [...]

Maybe something along the lines of "wait 24 hours after any discussion
regarding the current revision has settled before publishing the next
revision" would be a good guideline even if it's not included in this 
patch?

But I'm realising now that that is probably outside of the scope of
this specific change so apologies for what was likely me stepping into 
the realm of bike shedding. 

I didn't mean to hold up the patch and I'm happy with v4.

Re: Re* [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Jacob Abel <jacobabel@nullpo.dev> writes:

> Maybe something along the lines of "wait 24 hours after any discussion
> regarding the current revision has settled before publishing the next
> revision" would be a good guideline even if it's not included in this 
> patch?

Perhaps.

Usually after an iteration or two of a topic, it will become clear
who are available and interested in the topic, and "Wait and give
them enough time to respond to what you write" would become the most
appropriate guideline at that point.

But for new contributors and for more experienced ones alike, the
interest level from others is much harder to assess for the first
iteration, until everybody on the list has chance to notice and get
interested in the topic.  So "wait at least for 24 hours after
posting the first iteration" would be a good guideline for those who
do not know who on the list are the likely candidates to be
interested and know how quick their responses usually have
historically been.

A mistake I have often seen by new folks is to send their v2 soon
after they get a single minor response to their v1, without saying
why they are sending v2 at the time (e.g. "I am only fixing the typo
that was pointed out").  It takes much shorter time to come up with
a response to point out a typo or two in the proposed log message
than giving a deeper analysis, which may only come after a few
iterations of such trivial changes.

Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Linus Arver]

Junio C Hamano <gitster@pobox.com> writes:

> ...
>
> The inter/range-diff with my v3 was totally useless, but here is to
> show three minor edits I made to Linus's version I am responding to.
>
>  * Simplify parenthesized "because it may be the case that".
>
>  * As if you were "a" reviewer, as we do not designate "the
>    reviewer(s)" to a patch.  Anybody can (volunteer to) be a
>    reviewer for a patch, and you can be, too.
>
>  * Stress that a single polished patch that comes later (because it
>    took time to polish) is vastly preferred than flurry of "oops
>    this is better" updates.

All very reasonable and sensible. LGTM, thanks!

Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Linus Arver <linusa@google.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> ...
>>
>> The inter/range-diff with my v3 was totally useless, but here is to
>> show three minor edits I made to Linus's version I am responding to.
>>
>>  * Simplify parenthesized "because it may be the case that".
>>
>>  * As if you were "a" reviewer, as we do not designate "the
>>    reviewer(s)" to a patch.  Anybody can (volunteer to) be a
>>    reviewer for a patch, and you can be, too.
>>
>>  * Stress that a single polished patch that comes later (because it
>>    took time to polish) is vastly preferred than flurry of "oops
>>    this is better" updates.
>
> All very reasonable and sensible. LGTM, thanks!

Thanks.

Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Torsten Bögershausen]

On Thu, Jul 27, 2023 at 05:43:17PM -0700, Junio C Hamano wrote:
> Finding mistakes in and improving your own patches is a good idea,
> but doing so too quickly is being inconsiderate to reviewers who
> have just seen the initial iteration and taking their time to review
> it.  Encourage new developers to perform such a self review before
> they send out their patches, not after.  After sending a patch that
> they immediately found mistakes in, they are welcome to comment on
> them, mentioning what and how they plan to improve them in an
> updated version, before sending out their updates.

That's all good, no possible improvements from my side.
However, a possible question below.

[]

> +Please give reviewers enough time to process your initial patch before
> +sending an updated version. That is, resist the temptation to send a new
> +version immediately, because others may have already started reviewing
> +your initial version.
> +
> +While waiting for review comments, you may find mistakes in your initial
> +patch, or perhaps realize a different and better way to achieve the goal
> +of the patch. In this case you may communicate your findings to other
> +reviewers as follows:
> +
> + - If the mistakes you found are minor, send a reply to your patch as if
> +   you were a reviewer and mention that you will fix them in an
> +   updated version.
> +
> + - On the other hand, if you think you want to change the course so
> +   drastically that reviews on the initial patch would be a waste of
> +   time (for everyone involved), retract the patch immediately with
> +   a reply like "I am working on a much better approach, so please
> +   ignore this patch and wait for the updated version."
> +
(That's all good)


> +Now, the above is a good practice if you sent your initial patch
> +prematurely without polish.  But a better approach of course is to avoid
> +sending your patch prematurely in the first place.

That is of course a good suggestion.
I wonder, how much a first time contributor knows about "polishing",
in the Git sense ?
From my experience, the polishing is or could be a learning process,
which needs interaction with the reviewers.
Would it make sense to remove the sentences above and ask people
to mark their patch with RFC ?

Or is this all too much bikeshedding, IOW I am happy with V4 as is.

Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much

[Junio C Hamano]

Torsten Bögershausen <tboegi@web.de> writes:

>> +While waiting for review comments, you may find mistakes in your initial
>> +patch, or perhaps realize a different and better way to achieve the goal
>> +of the patch. In this case you may communicate your findings to other
>> +reviewers as follows:
>> +
>> + - If the mistakes you found are minor, send a reply to your patch as if
>> +   you were a reviewer and mention that you will fix them in an
>> +   updated version.
>> +
>> + - On the other hand, if you think you want to change the course so
>> +   drastically that reviews on the initial patch would be a waste of
>> +   time (for everyone involved), retract the patch immediately with
>> +   a reply like "I am working on a much better approach, so please
>> +   ignore this patch and wait for the updated version."
>> +
> (That's all good)
>
>
>> +Now, the above is a good practice if you sent your initial patch
>> +prematurely without polish.  But a better approach of course is to avoid
>> +sending your patch prematurely in the first place.
>
> That is of course a good suggestion.
> I wonder, how much a first time contributor knows about "polishing",
> in the Git sense ?

I do not know if "without polish" has any strong "Git sense" to
begin with.  The actions the contributor would have done, referred
to as "the above", are the result of re-reading their own patches
and re-thinking their own approaches, which led them to discover
fixes and alternative solutions, and I was hoping that "without
polish" would be understood by anybody to mean "a version that did
not go through such proofreading" without knowing much about how we
develop our patches.

I am OK to just say "sent your initial patch prematurely" and
without "without polish".  I do think it would make the resulting
text encourage less strongly to proofread their own patches before
sending them, but if you think "polish" may not be understood, I am
fine with such a copyediting.  Or using some alternative phrases is
also fine, if it improves our chances to be understood better.

> From my experience, the polishing is or could be a learning process,
> which needs interaction with the reviewers.

Yes, once they see what valuable insight reviewers offer, in their
next topic, they will learn to stop and think before sending the
fresh-off-the-press version without sleeping on it a bit.

> Would it make sense to remove the sentences above and ask people
> to mark their patch with RFC ?

I doubt it.  Nobody will stay newbie forever.  If they do not know
what kind of flaws to look for and how to find them themselves in
their work, that is perfectly OK and they can just send a regular
PATCH.  A reviewer hopefully will notice and point out that it is
not yet beyond RFC quality if that is the case, but this document
should not be suggesting that before seeing their work ;-)

> Or is this all too much bikeshedding, IOW I am happy with V4 as is.

Thanks.

Re: [PATCH] MyFirstContribution: refrain from self-iterating too much

[Sean Allred]

First review on this list; let me know if I've missed any decorum :-)

Junio C Hamano <gitster@pobox.com> writes:
> +Of course, you still may spot mistakes and rooms for improvements
> +after you sent your initial patch.  Learn from that experience to
> +make sure that you will do such a self-review _before_ sending your
> +patches next time.  You do not have to send your patches immediately
> +once you finished writing them.  It is not a race.  Take your time
> +to self-review them, sleep on them, improve them before sending them
> +out, and do not allow you to send them before you are reasonably
> +sure that you won't find more mistakes in them yourself.

Something of a nit:

    do not allow you to send them...

should be

    do not allow yourself to send them...

You're also using 'them' a *lot* which took me for a tumble my first
read-through. I lost track of what 'them' actually was. Since this
documentation is especially likely to be read by those who are already
confused by the process, it may be beneficial to be more explicit at
some points:

    ...

    patches next time. You do not have to send your patches immediately
    once you finished writing them. It is not a race. Take your time to
    review your own patches, sleep on them, and improve them. Do not
    allow yourself to send out your patches for review before you are
    reasonably sure you won't find more mistakes in them yourself.

--

Thanks for all the work you do on this list; it's much appreciated.

-Sean

--
Sean Allred