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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
