Linux kernel review tags (Re: [PATCH 0/3] Fix Thumb-2 undef handling for mixed-arch kernels)

[Dave Martin]

On Thu, Aug 18, 2011 at 10:18:10PM -0400, Nicolas Pitre wrote:
> On Thu, 18 Aug 2011, Dave Martin wrote:
> 
> > On Wed, Aug 17, 2011 at 6:24 PM, Nicolas Pitre <nicolas.pitre at linaro.org> wrote:
> > > On Wed, 17 Aug 2011, Dave Martin wrote:
> > >
> > >> Acked-by = This patch is definitely right, or I fully agree with the
> > >>       patch and trust the author's judgement ("I will share
> > >>       responsibility for the correctness and appropriateness of this
> > >>       patch").  This implies Reviewed-by.
> > >>       Normally an ack shouldn't
> > >>       get added unless the acker is confident that the patch is
> > >>       adequately tested (where the level of testing deemed adequate
> > >>       depends on the complexity of the patch)  Again, this may rely on
> > >>       judgement of the comptence of the author and the other
> > >>       reviewers.
> > >>
> > >> Reviewed-by = This patch looks correct and appropriate and I judge it
> > >>       ok to merge, but I assume the author knows what they're
> > >>       doing, and I don't necessarily take responsibility for the
> > >>       change.
> > >
> > > I think some aspects of the above two are mixed up.
> > >
> > > Normally, ACK == acknowledgement i.e. "I conceptually agree with the
> > > patch", but that doesn't necessarily mean that it was reviewed
> > > thoroughly.  In other words, this quite matches your definition, but
> > > does not imply a Reviewed-by, and that assumes the author knows what
> > > they're doing.
> > >
> > > Reviewed-by means that you did review the patch content in details,
> > > whether or not the author knows what they're doing.  A Reviewed-by
> > > obviously implies an Acked-by.
> > 
> > Interesting... I thought there was a chance I was getting this wrong.
> > 
> > My impression was that an Ack carries more weight with upstream
> > maintainers when it comes to merging; but does it instead depend on
> > _who_ the tag comes from?  (i.e., if an experienced and well-known
> > person takes a cursory glance at the patch and the review that's gone
> > on and Acks it, this may carry more weight than a Reviewed-by by a
> > less well-known person?)
> 
> Absolutely.
> 
> And the more experienced a person might be, the more patches that person 
> might be expected to look at.  So it is normal for such person to look 
> at the purpose and general design of a patch only, while trusting the 
> author to get the details right.  Hence the acked-by tag.
> 
> This is also where the coding style get important as it is possible for 
> a reviewer to look at the patch and get a feel for that general design 
> more easily.
> 
> A Reviewed-by is meant to be more thorough.  See the definition from Ted 
> Tso here:
> 
> http://kerneltrap.org/Linux/Introducing_Reviewed-by_Tags
> 
> But it is true that the value of any such tag is pondered by the 
> reputation of the person providing it, and that reputation is usually 
> based on the perceived quality of the code that person provided in the 
> past.

Useful advice, thanks

---Dave