Re: [PATCH v5 0/4] commit: Add commit.verbose configuration

[Junio C Hamano <gitster@pobox.com>]

Jeremiah Mahler <jmmahler@gmail.com> writes:

> On Fri, Jun 13, 2014 at 11:49:10AM -0500, Caleb Thompson wrote:
> ...
>> > The patches look good, they apply clean ('git am'), and all tests pass.
>> >
>> > Reviewed-by: Jeremiah Mahler <jmmahler@gmail.com>
>> 
>> So that I'm clear on the etiquitte, is it appropriate for me to add this
>> Reviewed-by line to the commit messages at this point, provided that the
>> patches don't change?
>
> I am not sure of the etiquette either.  But personally, since I hardly
> contributed anything, I don't think it is necessary to put my tag in
> your patches.

It is correct that it is not necessary for Caleb to resend these
patches if it is done only to add "Reviewed-by:" from you (but it
does not hurt to do so, either).

In the review process, "Reviewed-by:" sent for a patch by one of the
trusted reviewers will reduce workload from other reviewers because
there will be one less reason to read such a patch [*1*].  A
corollary to this is that other reviewers will ignore an extra
"Reviewed-by:" by somebody whose review quality is still unknown, so
it will not hurt.

Your reading others patches and commenting on is appreciated very
much, and sending a "Reviewed-by:" is perfectly fine.  As you gain
experience and as others see your review comments more and more,
people will start trusting your reviews.  Everybody begins at a
"novice" state, after all ;-)

By the way, speaking of netiquette, please refrain from using
"Mail-Followup-To:" while working on this list [*2*].

Thanks.

[Footnotes]

*1* We usually read a patch for one of the two reasons: either the
reviewer personally finds what the patch wants to do interesting and
worthwhile, and wants to make sure it is done in the right way.  Or
the reviewer thinks that applying the patch is detrimental to the
overall project, perhaps the design and/or the implementation is
wrong, and point the problems out.  A "Reviewed-by" from a trusted
reviewer will allow other reviewers to simply skip/ignore the patch
if what it does is "Meh" to them, saying "I am not particularly
interested, but as long as it does not hurt, I would not be opposed,
and the other guy reviewed and says it would not hurt, and I tend to
trust his judgment."  The maintainer does not have the luxury of
skip/ignore such a patch because he needs to at least apply and test
the integration result, though ;-).

Now, who are the "trusted reviewers"?  Anybody who is knows s/he is
one of them ;-)

*2* http://thread.gmane.org/gmane.comp.version-control.git/165477/focus=165549