[libvirt] [RFC] Adopting 'Tested-by' tag (and probably other tags)?

[Daniel P. Berrange]

On Wed, Apr 26, 2017 at 08:14:31AM -0400, John Ferlan wrote:
> Still for the most part, I would "hope" that an S-o-B could come with
> the implicit expectation that they've run "make check syntax-check".
> Similarly a reviewer would I think for a majority of what they review,
> apply the patches and at least build them. Many times when I have
> patches that I'm not sure will trip up Coverity - I'll run them through
> a test coverity build too, but I don't attribute that ever as that seems
> to draw unwanted attention sometimes...

In general S-o-B is /only/ about stating your compliance with the
legal side of contribution rules. ie confirming you have the right
to submit the patch. Whether they've complied with coding guidelines
is tangential.

> As for Reviewed-by - I do see that being something that's a bit more
> useful. Although how does one attribute R-b for trivial or build breaker
> patches?

S-o-B is considered to be a superset of Reviewed-by.

The person who merges the patch to git master would add their own
S-o-B line to indicate that they have the right to merge to it,
and implicitly that they've reviewed it.

IOW, you wouldn't have S-o-B and R-b  for the same person on the
same patch - just S-o-B is sufficient.

This makes it trivial for the person merging the patch to git,
since they simply can do  git am <the patch> and then
"git commit --amend -s" to add their S-o-B

> Since they aren't "automagically added", that means adding them requires
> the Submittor to actually make the effort. So if that's then the case
> that every submit has to have them, how does that get enforced?

It would be possible to write a git hook that refused a patch unless
the patch has a S-o-B from the person doing the push.

Likewise the git hook can validate that the patch author has also
proivded an S-o-B.

> What happens if one forgets or one consistently doesn't provide the
> tags? Is their push privilege taken away? IOW, what's the penalty for
> not following the accepted community rule?

> Again, I'm not against this, but sometimes getting *any* commit message
> for a patch is a struggle for some! Adding tags could be torturous ;-)

Dealing with S-o-B is trivial, since it just means adding -s flag to
git. The other tags are more manual, but not that much work.

The reviewed-by tags serve two purposes - one they give an indication to
the subsystem maintainer that the code has a certain level of review and
thus might be ready for merging, and two they give a historic record of
who did what. In our model with much broader set of committers, I don't
think the reviewed-by gives much benefit, so it just becomes a record of
who did what - which is already something present in the mailing list.

Personally I'd just keep things simple and only require a S-o-B from the
patch author, and the person committing. If people want to add other
tags fine, but I certaily wouldn't enforce anything other than S-o-B


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