On 20/11/2020 17:52, Junio C Hamano wrote: > A review exchange may begin with a reviewer asking "what did you > mean by this phrase in your log message (or here in the doc)?", the > author answering what was meant, and then the reviewer saying "ah, > that is what you meant---then the flow of the logic makes sense". > > But that is not the happy end of the story. New contributors often > forget that the material that has been reviewed in the above exchange > is still unclear in the same way to the next person who reads it, > until it gets updated. Yes! > While we are in the vicinity, rephrase the verb "request" used to > refer to comments by reviewers to "suggest"---this matches the > contrast between "original" and "suggested" that appears later in > the same paragraph, and more importantly makes it clearer that it is > not like authors are to please reviewers' wishes but rather > reviewers are merely helping authors to polish their commits. > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > > * Something along this line, in a more condensed form, may also > want to be in SubmittingPatches, but let's start with a longer > form that is easier to discuss the intent of the addition to see > if it is a good idea. I've seen a patch that got reviewed > falling thru the cracks without producing a v2 too many times. > > Documentation/MyFirstContribution.txt | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git c/Documentation/MyFirstContribution.txt w/Documentation/MyFirstContribution.txt > index 60eed5edcd..bac4997e39 100644 > --- c/Documentation/MyFirstContribution.txt > +++ w/Documentation/MyFirstContribution.txt > @@ -1143,11 +1143,24 @@ After a few days, you will hopefully receive a reply to your patchset with some > comments. Woohoo! Now you can get back to work. > > It's good manners to reply to each comment, notifying the reviewer that you have > -made the change requested, feel the original is better, or that the comment > +made the change suggested, feel the original is better, or that the comment > inspired you to do something a new way which is superior to both the original > and the suggested change. This way reviewers don't need to inspect your v2 to > figure out whether you implemented their comment or not. > > +Reviewers may ask you about what you wrote in the patchset, either in > +the proposed commit log message or in the changes themselves. You > +should answer these questions in your response messages, but often the > +reason why reviewers asked these questions to understand what you meant > +to write is because your patchset needed clarification to be understood. Perhaps a paragraph break here? > +Do not be satisfied by just answering their questions in your response > +and hear them say that they now understand what you wanted to say. > +Update your patches to clarify the points reviewers had trouble with, > +and prepare your v2; the words you used to explain your v1 to answer > +reviewers' questions may be useful thing to use. Your goal is to make > +your v2 clear enough so that it becomes unnecessary for you to give the > +same explanation to the next person who reads it. > + > If you are going to push back on a comment, be polite and explain why you feel > your original is better; be prepared that the reviewer may still disagree with > you, and the rest of the community may weigh in on one side or the other. As Is this also worth mentioning in SubmittingPatches? With or without the paragraph split Reviewed-by: Philip Oakley <philipoakley@iee.email>
