Re: [PATCH v2 1/4] git-p4: add optional type specifier to gitConfig reader

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 3, 2015, 21:31 UTC
Message-ID: <xmqqio7ri40h.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <6FAAE139-9010-4C68-AA97-2739E9A09564@gmail.com>

Lars Schneider <larsxschneider@gmail.com> writes:

> In case I agree with a reviewer. What is the more appropriate action?
> A response like the one above or a new role that includes the change
> right away? I don’t want to spam the list with lots of tiny changes…

Responding to review comment like you did with "will do" is perfectly fine.

When you do think you will (eventually) want to send an updated patch, there are things other than "will do" that you can say. "I understand what you said, but I am not sure how exactly to make this better. Perhaps lend me a help?" is good, too.

An explanation, followed by "ok?", in response to "it is unclear what you are doing here" (commenting on code) or to "I cannot understand what this is trying to say" (commenting on log message), is problematic because your intention becomes ambiguous.

The reviewers are rarely saying "I do not understand; educate me." but "I do not understand, and it is likely many others don't, too. Make it more easily understandable." is what they mean.

An explanation with "ok?" can be taken as a sign that you mistook the review comment as "educate me".

	What I meant was ....  Do you think commenting the code here
	with the above description is good enough?  Or do you think
	of a way to restructure the code itself to be more self
	evident?

or something like that may be a way to avoid the ambiguity.

Thanks.