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@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