Linux kernel review tags (Re: [PATCH 0/3] Fix Thumb-2 undef handling for mixed-arch kernels)
dave.martin at linaro.org
Fri Aug 19 08:48:44 UTC 2011
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?)
> 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:
> 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
Useful advice, thanks
More information about the linaro-dev