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]

 



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