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@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 there?
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 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.
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@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.
Cheers ---Dave