[Ksummit-discuss] [CORE TOPIC] Recruitment (Reviewers, Testers, Maintainers, Hobbyists)
Josh Poimboeuf
jpoimboe at redhat.com
Fri Jul 10 18:14:09 UTC 2015
On Wed, Jul 08, 2015 at 02:53:15PM -0400, Steven Rostedt wrote:
> I personally don't trust a Reviewed-by tag much, as I sometimes see
> them appear without any comments.
>
> I was thinking of writing a perl script that would read my LKML archive
> and somewhat intelligently looking at people who replied to patches,
> that also has snippets of the patch in the reply, and counting them. I
> think that would be a more accurate use of real reviewers than just the
> Reviewed-by tag.
I'm relatively new to lkml so my perceptions might be skewed. But it
seems to me that, for non-trivial patches, the Reviewed-by tag is pretty
much useless as a metric. From what I can tell, the biggest problems
with relying on Reviewed-by to get meaningful statistics are:
1. It's self-reported by the reviewer.
Maybe a reviewer gave some hugely valuable feedback in versions 1-4
of the patch, but wasn't around to provide the Reviewed-by tag for
the final v6.
Or maybe they're just focused on the review, and forget or don't care
to ask for credit for it at the time by adding a tag.
Or maybe they're a maintainer who used Signed-off-by instead (but
Signed-off-by doesn't necessarily imply a review).
Also, as others have mentioned, it creates the ability to game the
system by saying you've reviewed a patch when you really haven't.
2. It's too broad of a statement. It requires the reviewer to make an
official certification that they fully understand and approve of the
*entire* patch. That's certainly a useful statement, but:
A reviewer might review only part of the patch, and provide some
useful comments for just the part of the patch they reviewed. But
they don't want to take responsibility for saying, to quote Steven,
"I took enough effort to understand every part of the patch as if I
wrote it myself."
Or maybe they didn't carefully review the patch, but instead added
something valuable to the discussion which improved the patch in a
future version.
So it denies credit to all the other people who provided useful
comments along the way.
Review comments such as "here's a problem with your code" or "have you
thought about doing this instead?" can often be more useful a comment
than "your code looks good". Maybe it would be a good idea to
acknowledge those people who give valuable feedback, in all its forms.
Maybe a new tag would be useful? Something which:
a) is reported by the patch author and/or maintainer, since they're in
the best position to keep track of useful comments across versions;
and
b) means something like "I'm grateful to this person for giving some
valuable feedback".
--
Josh
More information about the Ksummit-discuss
mailing list