* [PATCH] MyFirstContribution: refrain from self-iterating too much @ 2023-01-22 1:51 Junio C Hamano 2023-01-22 7:11 ` Torsten Bögershausen 2023-01-23 1:47 ` [PATCH] " Sean Allred 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2023-01-22 1:51 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] MyFirstContribution: refrain from self-iterating too much 2023-01-22 1:51 [PATCH] MyFirstContribution: refrain from self-iterating too much Junio C Hamano @ 2023-01-22 7:11 ` Torsten Bögershausen 2023-01-22 16:01 ` Junio C Hamano 2023-01-23 1:47 ` [PATCH] " Sean Allred 1 sibling, 1 reply; 20+ messages in thread From: Torsten Bögershausen @ 2023-01-22 7:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] MyFirstContribution: refrain from self-iterating too much 2023-01-22 7:11 ` Torsten Bögershausen @ 2023-01-22 16:01 ` Junio C Hamano 2023-01-22 17:14 ` Junio C Hamano 2023-01-23 4:18 ` [PATCH v2] " Junio C Hamano 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2023-01-22 16:01 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: git 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] MyFirstContribution: refrain from self-iterating too much 2023-01-22 16:01 ` Junio C Hamano @ 2023-01-22 17:14 ` Junio C Hamano 2023-01-23 4:18 ` [PATCH v2] " Junio C Hamano 1 sibling, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2023-01-22 17:14 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: git 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] MyFirstContribution: refrain from self-iterating too much 2023-01-22 16:01 ` Junio C Hamano 2023-01-22 17:14 ` Junio C Hamano @ 2023-01-23 4:18 ` Junio C Hamano 2023-01-23 17:58 ` Torsten Bögershausen 1 sibling, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2023-01-23 4:18 UTC (permalink / raw) To: git; +Cc: Torsten Bögershausen 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] MyFirstContribution: refrain from self-iterating too much 2023-01-23 4:18 ` [PATCH v2] " Junio C Hamano @ 2023-01-23 17:58 ` Torsten Bögershausen 2023-07-19 17:04 ` [PATCH v3] " Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Torsten Bögershausen @ 2023-01-23 17:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3] MyFirstContribution: refrain from self-iterating too much 2023-01-23 17:58 ` Torsten Bögershausen @ 2023-07-19 17:04 ` Junio C Hamano 2023-07-27 23:14 ` Linus Arver 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2023-07-19 17:04 UTC (permalink / raw) To: git; +Cc: Torsten Bögershausen 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3] MyFirstContribution: refrain from self-iterating too much 2023-07-19 17:04 ` [PATCH v3] " Junio C Hamano @ 2023-07-27 23:14 ` Linus Arver 2023-07-28 0:25 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Linus Arver @ 2023-07-27 23:14 UTC (permalink / raw) To: Junio C Hamano, git; +Cc: Torsten Bögershausen 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--- ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3] MyFirstContribution: refrain from self-iterating too much 2023-07-27 23:14 ` Linus Arver @ 2023-07-28 0:25 ` Junio C Hamano 2023-07-28 0:43 ` [PATCH v4] " Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2023-07-28 0:25 UTC (permalink / raw) To: Linus Arver; +Cc: git, Torsten Bögershausen 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 0:25 ` Junio C Hamano @ 2023-07-28 0:43 ` Junio C Hamano 2023-07-28 2:07 ` Jacob Abel ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Junio C Hamano @ 2023-07-28 0:43 UTC (permalink / raw) To: git; +Cc: Linus Arver, Torsten Bögershausen 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 0:43 ` [PATCH v4] " Junio C Hamano @ 2023-07-28 2:07 ` Jacob Abel 2023-07-28 5:10 ` Junio C Hamano 2023-07-28 2:08 ` Linus Arver 2023-07-28 21:21 ` Torsten Bögershausen 2 siblings, 1 reply; 20+ messages in thread From: Jacob Abel @ 2023-07-28 2:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Linus Arver, Torsten Bögershausen 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 2:07 ` Jacob Abel @ 2023-07-28 5:10 ` Junio C Hamano 2023-07-28 15:42 ` Re* " Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2023-07-28 5:10 UTC (permalink / raw) To: Jacob Abel; +Cc: git, Linus Arver, Torsten Bögershausen 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re* [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 5:10 ` Junio C Hamano @ 2023-07-28 15:42 ` Junio C Hamano 2023-07-29 2:12 ` Jacob Abel 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2023-07-28 15:42 UTC (permalink / raw) To: Jacob Abel; +Cc: git, Linus Arver, Torsten Bögershausen 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: Re* [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 15:42 ` Re* " Junio C Hamano @ 2023-07-29 2:12 ` Jacob Abel 2023-07-31 15:25 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Jacob Abel @ 2023-07-29 2:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Linus Arver, Torsten Bögershausen 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Re* [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-29 2:12 ` Jacob Abel @ 2023-07-31 15:25 ` Junio C Hamano 0 siblings, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2023-07-31 15:25 UTC (permalink / raw) To: Jacob Abel; +Cc: git, Linus Arver, Torsten Bögershausen 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 0:43 ` [PATCH v4] " Junio C Hamano 2023-07-28 2:07 ` Jacob Abel @ 2023-07-28 2:08 ` Linus Arver 2023-07-28 5:10 ` Junio C Hamano 2023-07-28 21:21 ` Torsten Bögershausen 2 siblings, 1 reply; 20+ messages in thread From: Linus Arver @ 2023-07-28 2:08 UTC (permalink / raw) To: Junio C Hamano, git; +Cc: Torsten Bögershausen 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! ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 2:08 ` Linus Arver @ 2023-07-28 5:10 ` Junio C Hamano 0 siblings, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2023-07-28 5:10 UTC (permalink / raw) To: Linus Arver; +Cc: git, Torsten Bögershausen 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 0:43 ` [PATCH v4] " Junio C Hamano 2023-07-28 2:07 ` Jacob Abel 2023-07-28 2:08 ` Linus Arver @ 2023-07-28 21:21 ` Torsten Bögershausen 2023-07-28 23:00 ` Junio C Hamano 2 siblings, 1 reply; 20+ messages in thread From: Torsten Bögershausen @ 2023-07-28 21:21 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Linus Arver, jacobabel 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4] MyFirstContribution: refrain from self-iterating too much 2023-07-28 21:21 ` Torsten Bögershausen @ 2023-07-28 23:00 ` Junio C Hamano 0 siblings, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2023-07-28 23:00 UTC (permalink / raw) To: Torsten Bögershausen; +Cc: git, Linus Arver, jacobabel 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] MyFirstContribution: refrain from self-iterating too much 2023-01-22 1:51 [PATCH] MyFirstContribution: refrain from self-iterating too much Junio C Hamano 2023-01-22 7:11 ` Torsten Bögershausen @ 2023-01-23 1:47 ` Sean Allred 1 sibling, 0 replies; 20+ messages in thread From: Sean Allred @ 2023-01-23 1:47 UTC (permalink / raw) To: Junio C Hamano; +Cc: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-07-31 15:25 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-22 1:51 [PATCH] MyFirstContribution: refrain from self-iterating too much Junio C Hamano 2023-01-22 7:11 ` Torsten Bögershausen 2023-01-22 16:01 ` Junio C Hamano 2023-01-22 17:14 ` Junio C Hamano 2023-01-23 4:18 ` [PATCH v2] " Junio C Hamano 2023-01-23 17:58 ` Torsten Bögershausen 2023-07-19 17:04 ` [PATCH v3] " Junio C Hamano 2023-07-27 23:14 ` Linus Arver 2023-07-28 0:25 ` Junio C Hamano 2023-07-28 0:43 ` [PATCH v4] " Junio C Hamano 2023-07-28 2:07 ` Jacob Abel 2023-07-28 5:10 ` Junio C Hamano 2023-07-28 15:42 ` Re* " Junio C Hamano 2023-07-29 2:12 ` Jacob Abel 2023-07-31 15:25 ` Junio C Hamano 2023-07-28 2:08 ` Linus Arver 2023-07-28 5:10 ` Junio C Hamano 2023-07-28 21:21 ` Torsten Bögershausen 2023-07-28 23:00 ` Junio C Hamano 2023-01-23 1:47 ` [PATCH] " Sean Allred
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).