Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH] builtin-remote: better handling of multiple remote HEADs

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:10

Jeff King [off-list ref] writes:
Which made me think of something else, with all of this talk about
reviewers that has been going on. Junio is actually in a little bit of a
special position with small changes (like style issues) to say "I'll
apply this, but tweak these changes".
It is not that I am special.

What is special is an otherwise obviously good patch with a few trivial
mistakes that I can fix locally without worrying the fix-up may be wrong.
It is not even per author, it is per patch, and it is a rare exception.

Often, I notice these things *after* I applied and reviewed the results,
so it already is in my work area.  I then judge the tradeoff between an
extra round (which as you stated needs another fresh review, patch
application and testing here) and the possibility that I may make a silly
mistake myself while attempting a fix-up (such a mistake by me will not
be seen on the list and others do not have chance to catch them).

For this reason, I try to keep these "will fix up no need for resend" to
the minimum and only to the most trivial cases.
... But the rest of us are stuck
saying "I would change this one line" to the list; then either:

  - the original submitter re-rolls the patch, which takes their time
    and everyone else's time to look at the new patch, see that it is
    trivially changed, etc

    or

  - Junio has to read the followup comments, then go back and find the
    spot in the original patch to mark it up.
A third option is:

	"I would change this and that" review comment message, followed by
	a separate message "Here is how I would have done it", addressed
	To: the original submitter (with in-body From: line), Cc: to the
	list and me.

The original submitter can verify the latter one, and either agree to or
disagree with it.  If the reroll is good, then I can just pick it up.  I
think you have done that in the past yourself, and the process made my
life a lot easier.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help