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é <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
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 :|
