Linux kernel review tags (Re: [PATCH 0/3] Fix Thumb-2 undef handling for mixed-arch kernels)
dave.martin at linaro.org
Wed Aug 17 10:44:58 UTC 2011
On Tue, Aug 16, 2011 at 05:43:08PM +0100, Tixy wrote:
> On Tue, 2011-08-16 at 17:06 +0100, Dave Martin wrote:
> > On Tue, Aug 16, 2011 at 3:58 PM, Tixy <tixy at yxit.co.uk> wrote:
> I've been re-reading Documentation/SubmittingPatches to try and work out
> when it's appropriate to use Acked-by, but it seems a bit woolly -
> mostly for maintainers?
Actually I asked the question semi-automatically since you are CC'd
on this series -- because I was originally touching kprobes code,
which you have in-depth understanding of.
But now the series doesn't touch that code any more, reviewed-by
makes more sense.
Here's my own interpretation of the tags, based on how they appear
to be used (obviously, this doesn't override SubmittingPatches; it's
my additional interpretation)
I could of course be wrong in some aspects.
Any comments on this from the more experienced kernel developers out
Signed-off-by = I take full responsibility for this patch,
and can maintain it in the author's absence if required.
In practice, this implies Acked-by.
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
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
Tested-by = I have built and tested the patch, and saw reasonable
evidence that it achieves the intended result and has no
harmful effects. I make no comment about contents of the patch,
and I don't necessarily take responsibility for the change.
In all cases, the tag may influence upstreams' judgement about
whether to merge the patch: adding a tag implies understanding of this,
and takes responsibility for the effect this has (wise or unwise)
on the decision to merge.
NA(C)K = Informal indication that this patch is definitely wrong
(not listed in SubmittingPatches, because if someone is that convinced
the patch is wrong, it isn't normally going to get merged without
rework... or the NACKer is themselves known by upstream to be
reliably untrustworthy, biased, or wrong. In that case, the
upstream maintainter effectively overrides the NACKer.
An Ack is normally not useful unless it comes from someone who is
themselves known to and trusted by the maintainer of the tree where
the patch will get merged, or unless it comes from the original
author or a maintainer of the code that was modified (since they are
normally the best people to judge). Acked-by: randomnewbie at hotmail.com
isn't likely to carry much weight. In fact, including blatantly
frivolous or fraudulent acks from people in your upstream patch
submissions may harm their chance of merging (and your reputation).
So for now, I don't often ack patches -- I generally do so when someone
touches code I modified/added, to indicate that I understand their change
well enough, and am confident enough that it is correct, that I would be
happy to share some responsiibility for their change and any resulting
impacts on the maintenance of the code I added.
In areas where I know I have and am known to have a good understanding
(such as Thumb-2 related issues), I may ack patches touching code that
is nothing to do with me, because in such cases the Ack has a useful
meaning for the upstream maintainer.
More information about the linaro-dev