GNU Tools Cauldron 2017 follow up: "Reviewed-by" etc.

[Thomas Schwinge <thomas@codesourcery.com>]

Hi!

Sending this to the "main" GNU tools mailing lists.  Of course, that's
not meant to exclude other tools.


Amongst other things ;-) at the GNU Tools Cauldron 2017 we discussed
whether having a "Reviewed-by" tag in the commit log might provide an
incentive for more people to invest time in patch review, and to get that
investment recognized more prominently/formally.  For the scheme used by
the Linux kernel, search for "Reviewed-by" on
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst>.
(Similar concepts to acknowledge time spent with PR triaging and other
things have also been discussed briefly, but I'll focus on this one
first.)

Generally, if adopting such a scheme (and I'm all for it!), I suggest
that we do not needlessly deviate from established processes, such as the
Linux kernel's, because remembering/looking up which project is doing
something in exactly which way is always confusing.


As David Edelsohn (as far I remember?) eloquently has put it, a lot of
things in the GNU tools community happened/improved just because somebody
started doing something.  ;-)

Now, with "Reviewed-by" it's actually two people who have to start doing
something: according to the Linux kernel's process, putting "Reviewed-by:
NAME <EMAIL>" into his email, the reviewer "indicates that the patch has
been reviewed and found acceptable according to the Reviewer's
Statement".  Then see "Reviewer's statement of oversight" on
<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst>:

| Reviewer's statement of oversight
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| 
| By offering my Reviewed-by: tag, I state that:
| 
|          (a) I have carried out a technical review of this patch to
|              evaluate its appropriateness and readiness for inclusion into
|              the mainline kernel.
| 
|          (b) Any problems, concerns, or questions relating to the patch
|              have been communicated back to the submitter.  I am satisfied
|              with the submitter's response to my comments.
| 
|          (c) While there may be things that could be improved with this
|              submission, I believe that it is, at this time, (1) a
|              worthwhile modification to the kernel, and (2) free of known
|              issues which would argue against its inclusion.
| 
|          (d) While I have reviewed the patch and believe it to be sound, I
|              do not (unless explicitly stated elsewhere) make any
|              warranties or guarantees that it will achieve its stated
|              purpose or function properly in any given situation.
| 
| A Reviewed-by tag is a statement of opinion that the patch is an
| appropriate modification of the kernel without any remaining serious
| technical issues.  Any interested reviewer (who has done the work) can
| offer a Reviewed-by tag for a patch.  This tag serves to give credit to
| reviewers and to inform maintainers of the degree of review which has been
| done on the patch.  Reviewed-by: tags, when supplied by reviewers known to
| understand the subject area and to perform thorough reviews, will normally
| increase the likelihood of your patch getting into the kernel.

So my question is, if I've gotten a patch reviewed by someone who is not
yet ;-) familiar with that new process, and I nevertheless want to
acknowledge their time invested in review by putting "Reviewed-by" into
the commit log, is it fine to do that if the reviewer just answered with
"OK" (or similar) instead of an explicit "Reviewed-by: NAME <EMAIL>"
statement?

That is, is it fine to assume that our current patch review's standard
"OK" (or similar) answer matches the more formal "Reviewer's statement of
oversight"?

If yes, then that will make it easier to start using this scheme,
avoiding the need for additional roundtrips à la: 'Does your patch
review's "OK" answer correspond to the "Reviewer's statement of
oversight" [...]'.

Maybe in the future, reviewers will then switch over to explicitly
stating "Reviewed-by: NAME <EMAIL>" -- or maybe not, because "OK" is just
so much easier to type...


Grüße
 Thomas