Re: [PATCH] SubmittingPatches: Document how to request a patch review tag

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 6, 2013, 09:01 UTC
Message-ID: <7vtxquofbd.fsf@alter.siamese.dyndns.org>
In-Reply-To: <50E92875.6080305@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 14 quoted lines

> On 01/04/2013 10:47 PM, Junio C Hamano wrote:
>> "Reviewed-by" is for those who are familiar with the part of the
>> system being touched to say "I reviewed this patch, it looks good",
>> and Michael indeed was involved in recent updates to the refs.c
>> infrastructure, so as he said in his message "it looks like I should",
>> it was the right thing to do.
>>
>> I do not think Michael was asking if that was the standard _thing_
>> to do; I think the question was if there was a standard _way_
>> (perhaps a tool) to send such a "Reviewed-by:" line.
>
> Junio is correct; I was just asking whether there was a particular email
> convention for adding a "Reviewed-by:" line.  It would be nice for this
> to be mentioned in the documentation.

Yeah, I wasn't exactly correct in that I was talking more about Acked-by than Reviewed-by, but they are morally very similar and the same argument applies to both.

In the particular case of your "refs.c" review, because you are not just familiar with the code, but essentially are the current owner of the code, Acked-by might have been even more appropriate than Reviewed-by, by the way.

Show 11 quoted lines

>> +If you are a reviewer and wish to add your Acked-by/Reviewed-by/Tested-by tag
>> +to a patch series under discussion (after having reviewed it or tested it
>> +of course!), reply to the author of the patch series, cc'ing the git mailing
>> +list.
>> +
>>  You can also create your own tag or use one that's in common usage
>>  such as "Thanks-to:", "Based-on-patch-by:", or "Mentored-by:".
>
> I don't think this is quite correct.  Such emails should be
> "reply-to-all" people who have participated in the thread, which might
> include more than just the patch author and the git mailing list.

That would be more helpful. In practice, I can pick these up either way, but Cc'ing everybody would be better as a common courtesy.

When the author resubmits an already reviewed patch with these Acks and Reviews for final application, these reviewers should be Cc'ed so that they can say "Huh? that is not the exact patch I reviewed. What is going on?".

Thanks for a review.