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

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

>> +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.
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to