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

[John Ferlan]

On 04/26/2017 05:10 AM, Daniel P. Berrange wrote:
> On Wed, Apr 26, 2017 at 11:06:54AM +0200, Kashyap Chamarthy wrote:
>> On Wed, Apr 26, 2017 at 09:35:47AM +0100, Daniel P. Berrange wrote:
>>> On Wed, Apr 26, 2017 at 09:55:50AM +0200, Kashyap Chamarthy wrote:
>>
>> [...]
>>
>>>> I realize that if it's not automated (via Git hooks or similar), it can
>>>> become "lossy", i.e. if Joe posts v1 of a patch, you give a 'Tested-by',
>>>> then there are two scenarios that immediately spring to mind:
>>>>
>>>>   (1) Joe respins a v2 to make some corrections, adds your 'Tested-by'
>>>>       tag, and whoever applies the patch picks it up -- all good.
>>>>
>>>>   (b) However, if a v2 was _not_ necessary, then whoever is applying the
>>>>       patch / series must remember to add the tag -- "lossy".
>>>
>>> I don't think that's a big deal really, QEMU has been doing this for years.
>>> If you post a vNNN of your patch, you are responsible for adding the tags.
>>
>> Nod; funnily enough, Markus Armbruster sort of mentioned this 'lossy'
>> notion with QEMU sometimes, with a note that: seasoned contributors /
>> maintainers are all fairly diligent, but sometimes mistakes happen.
>>
>>> When the sub-tree maintainer accepts your patch they add any outstanding
>>> tags, as well as their own S-o-B. This is little work compard to actually
>>> applying & testing the patch before pushing it
>>
>> Noted.  One of the reasons for sending this email was also my experience
>> interacting with upstream QEMU community, where the said tags are indeed
>> more rigorously used.
>>
>>>> Thoughts / remarks / rotten tomatoes welcome.

So not opposed to the concept - although like anything it could take
some period of "getting used to using it". I can certainly see it being
forgotten more frequently as we reach the end of each month and everyone
pushes to get changes in...

Since the libvirt community is far smaller, I think we'll rarely see T-b
other than if the testing is by the S-o-B. I can recall only reading a
few patch reviews over the last couple of years where someone noted that
they also tested the patch [series]. Although it does happen and should
then be attributed.

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

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?

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?

With QEMU I understand that's a bit easier with owners of sub-tree's
that can enforce that. But libvirt has multiple push capable
maintainers, so now each of them becomes responsible for that.

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 ;-)


>>>
>>> I'd like to see us formally adopt the signed-off-by approach for all
>>> patches as a mandatory thing, along with the associated contributor
>>> convenant.
>>
>> By "contributor covenant", I presume you're referring to:
>>
>>     http://contributor-covenant.org/
>>     http://contributor-covenant.org/version/1/4/code_of_conduct.txt
> 
> No, I mixed up the terms. What I actually meant was the Developer Certificate
> of Origin
> 
>    https://developercertificate.org/
> 
> The use of a 'Signed-off-by' indicates acceptance of this DCO (or similar)
> in projects which formally adopt S-o-B usage.
> 

Coming from someone who only more recently in his career has been an
open source contributor - I didn't realize the S-o-B usage is related to
knowing the existence of said document ;-)... It's not linked from our
hacking document either AFAICT.

John