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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03 Sep 2015, at 23:31, Junio C Hamano <gitster@xxxxxxxxx> wrote:

> Lars Schneider <larsxschneider@xxxxxxxxx> 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.

Thank you for the explanation! :-)

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]