Re: Giving your own patches your Reviewed-by

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Mar 12, 2025 at 11:03:04AM +0100, Philippe Mathieu-Daudé wrote:
> Hi Markus,
> 
> (Cc'ing Yi, Clément and Zhenzhong for commit eda4c9b5b3c)
> 
> On 12/3/25 10:45, Markus Armbruster wrote:
> > I stumbled over commits that carry the author's Reviewed-by.
> > 
> > There may be cases where the recorded author isn't the lone author, and
> > the recorded author did some meaningful review of the patch's parts that
> > are not theirs.  Mind that we do need all authors to provide their
> > Signed-off-by.
> > 
> > When the only Signed-off-by is from the recorded author, and there's
> > also their Reviewed-by, the Reviewed-by is almost certainly bogus.
> > 
> > Now, accidents happen, no big deal, etc., etc.  I post this to hopefully
> > help reduce the accident rate :)
> > 
> > Here's my quick & sloppy search for potentially problematic uses of
> > Reviewed-by:
> > 
> > $ git-log --since 'two years ago' | awk -F: '/^commit / { commit=$0 } /^Author: / { guy=$2 } /^    Reviewed-by: / { if ($2 == guy) { print commit; print guy } }'
> 
> 
> Explaining some commits where I'm mentioned:

> I posted a patch with my S-o-b; Richard took it, improved and reposted
> it with his S-o-b; I reviewed Richard's changes (and eventually merged).

That is totally fine, and an example of an expected scenario
that the simple shell 1-liner above can't eliminate.

> Is this workflow making sense and accepted? Otherwise what should
> we change? Maybe clarify along with the tags; or including all
> Message-Id could make this easier to track?

I don't see any problems with the commit messages as already written and
imho don't think we need to change it.

The main scenario that I would say is invalid is a commit containing
/only/ a matching rb/sob pair e.g.

 Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
 Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

as that is clearly either a "marking your own homework" case, or
someone has forgot to update the R-b/SoB tags.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|