git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
LALinus Arver <linusa@google.com>
Date
Jul 27, 2023, 23:14 UTC
Message-ID
<owlycz0deykz.fsf@fine.c.googlers.com>
In-Reply-To
<xmqq8rbbbzp2.fsf_-_@gitster.g>

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".
Show 5 quoted lines
> +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
Show 8 quoted lines
> +   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/
Show 6 quoted lines
> +
>  [[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---
 
Previous: Junio C HamanoNext: Junio C Hamano
Message 8 of 20 in “MyFirstContribution: refrain from self-iterating too much”
  1. MyFirstContribution: refrain from self-iterating too muchJunio C Hamano, Jan 22, 2023
  2. Torsten BögershausenJan 22, 2023
  3. Junio C HamanoJan 22, 2023
  4. Junio C HamanoJan 22, 2023
  5. MyFirstContribution: refrain from self-iterating too muchJunio C Hamano, Jan 23, 2023
  6. Torsten BögershausenJan 23, 2023
  7. MyFirstContribution: refrain from self-iterating too muchJunio C Hamano, Jul 19, 2023
  8. Linus ArverJul 27, 2023
  9. Junio C HamanoJul 28, 2023
  10. MyFirstContribution: refrain from self-iterating too muchJunio C Hamano, Jul 28, 2023
  11. Jacob AbelJul 28, 2023
  12. Junio C HamanoJul 28, 2023
  13. Re* [PATCH v4] MyFirstContribution: refrain from self-iterating too muchJunio C Hamano, Jul 28, 2023
  14. Jacob AbelJul 29, 2023
  15. Junio C HamanoJul 31, 2023
  16. Linus ArverJul 28, 2023
  17. Junio C HamanoJul 28, 2023
  18. Torsten BögershausenJul 28, 2023
  19. Junio C HamanoJul 28, 2023
  20. Sean AllredJan 23, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.