git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
* [PATCH] myFirstContribition: answering questions is not the end of the story
@ 2020-11-20 17:52 Junio C Hamano
  2020-11-20 21:42 ` Philip Oakley
  2020-11-21  2:54 ` Philippe Blain
  0 siblings, 2 replies; 7+ messages in thread
From: Junio C Hamano @ 2020-11-20 17:52 UTC (permalink / raw)
  To: git; +Cc: Emily Shaffer

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.

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@pobox.com>
---

 * 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.
+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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] myFirstContribition: answering questions is not the end of the story
  2020-11-20 17:52 [PATCH] myFirstContribition: answering questions is not the end of the story Junio C Hamano
@ 2020-11-20 21:42 ` Philip Oakley
  2020-11-20 22:34   ` Junio C Hamano
  2020-11-24 20:14   ` Emily Shaffer
  2020-11-21  2:54 ` Philippe Blain
  1 sibling, 2 replies; 7+ messages in thread
From: Philip Oakley @ 2020-11-20 21:42 UTC (permalink / raw)
  To: Junio C Hamano, git; +Cc: Emily Shaffer

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@pobox.com>
> ---
>
>  * 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>




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] myFirstContribition: answering questions is not the end of the story
  2020-11-20 21:42 ` Philip Oakley
@ 2020-11-20 22:34   ` Junio C Hamano
  2020-11-24 20:14   ` Emily Shaffer
  1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2020-11-20 22:34 UTC (permalink / raw)
  To: Philip Oakley; +Cc: git, Emily Shaffer

Philip Oakley <philipoakley@iee.email> writes:

> Is this also worth mentioning in SubmittingPatches?

Yes, see what I wrote immediately under three-dash lines.

My wish is that, once we know the complete message we want to send
with this longer document, we may see some volunteer to update
SubmittingPatches with a more condensed version. ;-)

Thanks.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] myFirstContribition: answering questions is not the end of the story
  2020-11-20 17:52 [PATCH] myFirstContribition: answering questions is not the end of the story Junio C Hamano
  2020-11-20 21:42 ` Philip Oakley
@ 2020-11-21  2:54 ` Philippe Blain
  2020-11-21 21:53   ` Junio C Hamano
  1 sibling, 1 reply; 7+ messages in thread
From: Philippe Blain @ 2020-11-21  2:54 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Emily Shaffer

Hi Junio,

Le ven. 20 nov. 2020, à 21 h 50, Junio C Hamano <gitster@pobox.com> a écrit :
>
> 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.
>
> 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@pobox.com>
> ---

I think this is a nice addition. However the patch title should probably
be prefixed "MyFirstContribution" and not "myFirstContribition" ;)

Cheers!

Philippe.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] myFirstContribition: answering questions is not the end of the story
  2020-11-21  2:54 ` Philippe Blain
@ 2020-11-21 21:53   ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2020-11-21 21:53 UTC (permalink / raw)
  To: Philippe Blain; +Cc: Git List, Emily Shaffer

Philippe Blain <levraiphilippeblain@gmail.com> writes:

> I think this is a nice addition. However the patch title should probably
> be prefixed "MyFirstContribution" and not "myFirstContribition" ;)

Will update.  Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] myFirstContribition: answering questions is not the end of the story
  2020-11-20 21:42 ` Philip Oakley
  2020-11-20 22:34   ` Junio C Hamano
@ 2020-11-24 20:14   ` Emily Shaffer
  2020-12-01  4:46     ` Junio C Hamano
  1 sibling, 1 reply; 7+ messages in thread
From: Emily Shaffer @ 2020-11-24 20:14 UTC (permalink / raw)
  To: Philip Oakley; +Cc: Junio C Hamano, git

On Fri, Nov 20, 2020 at 09:42:16PM +0000, Philip Oakley wrote:
> >  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?
Agreed.

> > +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.

Overall a point I really appreciate having written out here. I think
it's a good addition to this doc.

Reviewed-by: Emily Shaffer <emilyshaffer@google.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] myFirstContribition: answering questions is not the end of the story
  2020-11-24 20:14   ` Emily Shaffer
@ 2020-12-01  4:46     ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2020-12-01  4:46 UTC (permalink / raw)
  To: Emily Shaffer; +Cc: Philip Oakley, git

Emily Shaffer <emilyshaffer@google.com> writes:

>> > +to write is because your patchset needed clarification to be understood.
>> 
>> Perhaps a paragraph break here?
> Agreed.
> ...
> Overall a point I really appreciate having written out here. I think
> it's a good addition to this doc.
>
> Reviewed-by: Emily Shaffer <emilyshaffer@google.com>

Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-12-01  4:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 17:52 [PATCH] myFirstContribition: answering questions is not the end of the story Junio C Hamano
2020-11-20 21:42 ` Philip Oakley
2020-11-20 22:34   ` Junio C Hamano
2020-11-24 20:14   ` Emily Shaffer
2020-12-01  4:46     ` Junio C Hamano
2020-11-21  2:54 ` Philippe Blain
2020-11-21 21:53   ` Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git