public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Michael Haggerty <mhagger@alum•mit.edu>
Cc: Jason Holden <jason.k.holden.swdev@gmail•com>, git@vger•kernel.org
Subject: Re: [PATCH] SubmittingPatches: Document how to request a patch review tag
Date: Sun, 06 Jan 2013 01:01:42 -0800	[thread overview]
Message-ID: <7vtxquofbd.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <50E92875.6080305@alum.mit.edu> (Michael Haggerty's message of "Sun, 06 Jan 2013 08:32:05 +0100")

Michael Haggerty <mhagger@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.

  reply	other threads:[~2013-01-06  9:02 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-01 23:24 [PATCH 0/3] Update SubmittingPatches Junio C Hamano
2013-01-01 23:24 ` [PATCH 1/3] SubmittingPatches: who am I and who cares? Junio C Hamano
2013-01-01 23:24 ` [PATCH 2/3] SubmittingPatches: mention subsystems with dedicated repositories Junio C Hamano
2013-01-02  1:52   ` Jason Holden
2013-01-02  2:14     ` Junio C Hamano
2013-01-02 17:22       ` Junio C Hamano
2013-01-04 20:58         ` [PATCH] SubmittingPatches: Document how to request a patch review tag Jason Holden
2013-01-04 21:47           ` Junio C Hamano
2013-01-06  7:32           ` Michael Haggerty
2013-01-06  9:01             ` Junio C Hamano [this message]
2013-01-01 23:24 ` [PATCH 3/3] SubmittingPatches: remove overlong checklist Junio C Hamano
2013-01-02  9:08 ` [PATCH 0/3] Update SubmittingPatches Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vtxquofbd.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jason.k.holden.swdev@gmail$(echo .)com \
    --cc=mhagger@alum$(echo .)mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox