Re: [Qemu-devel] [PATCH] e1000: allow command-line selection of card model

[Andreas Färber]

Am 21.05.2014 12:22, schrieb Michael S. Tsirkin:
> On Wed, May 21, 2014 at 11:48:48AM +0200, Andreas Färber wrote:
>> Am 21.05.2014 11:25, schrieb Michael S. Tsirkin:
>>> On Wed, May 21, 2014 at 11:12:42AM +0200, Andreas Färber wrote:
>>>> Am 21.05.2014 11:04, schrieb Michael S. Tsirkin:
>>>>> On Wed, May 21, 2014 at 10:40:54AM +0200, Andreas Färber wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Am 20.05.2014 17:05, schrieb Gabriel L. Somlo:
>>>>>>> Allow selection of different card models from the qemu
>>>>>>> command line, to better accomodate a wider range of guests.
>>>>>>>
>>>>>>> Based-on-patch-by: Romain Dolbeau <address@hidden>
>>>>>>
>>>>>> If that patch carried a Signed-off-by line, you should retain it.
>>>>>
>>>>> Actually I think that would be confusing. Romain didn't sign off
>>>>> on *this* patch, he signed off on a previous one.
>>>>> A signature by Gabriel indicates Developer's Certificate of Origin 1.1
>>>>> which has an option to incorporate other's work - it
>>>>> does not seem to require signatures by these others.
>>>>
>>>> With the same argument you could drop anyone's Sob you get as a
>>>> maintainer.
>>>
>>> I could but it would not be nice to submitters, and it drops useful info
>>> (author's Sob).  So if someone thinks there's problematic code here and
>>> comes complaining, we want to be able to say "this code came from XYZ".
>>>
>>>
>>>> But the purpose of Sob is to track through whose hands a
>>>> patch went, not just who last touched it.
>>>
>>> Went untouched or mostly untouched.
>>> Did you bother checking?
>>> I looked and Romain's patch isn't very similar to this one.
>>>
>>>> My point here was that Based-on-patch-by is very unusual.
>>>
>>> What's the harm?
>>> Gabriel's just being nice and crediting other's work.
>>>
>>>> The alternative would be to leave the original From: Romain Dolbeau, his
>>>> Sob, then a [gsomlo: Dropped this, added that] followed by Sob.
>>>>
>>>> Cheers,
>>>> Andreas
>>>
>>> That's just asking submitter to do a lot of extra work,
>>> I don't see why would we place roadblocks in submitter's paths
>>> like this. Linux certainly does not and we didn't ask for this
>>> in the past.
>>>
>>> Further, the patch author in git will also be the original author then
>>> which is only fair if the patch is changed in very minor ways.
>>> In which case keeping the original Sob around *would* be right.
>>
>> Either the patch is based on the patch the submitter claims it is based
>> on, or it is not based on that patch.
>>
>> If it is, then the Sob should be retained because not doing so is
>> dropping useful information as you put it.
> 
> It isn't useful if most of the patch has been changed,
> because Sob without the patch itself has no meaning.
> 
>> You will find both ways, From
>> new and old+new Sob or From original and [], in git history, depending
>> on how much changed (which I have not checked here).
> 
> Saying "Based on patch" in commit log is common practice too.
> 
> You really can not redefine the meaning of Sob - it is
> widely understood to mean a very specific thing,
> Developer's certificate of origin 1.1 (which, by the way, qemu source
> really should include a copy of), which in particular says:
> 
>         (b) The contribution is based upon previous work that, to the best
>             of my knowledge, is covered under an appropriate open source
>             license and I have the right under that license to submit that
>             work with modifications, whether created in whole or in part
>             by me, under the same open source license (unless I am
>             permitted to submit under a different license), as indicated
>             in the file; or
> 
> so if you base upon legal previous work and can certify that, it is
> absolutely not required to track authors of that one and add their
> signatures. What if I copy code from virtual box's qemu? Would you make
> me troll their log for signatures?  When to do that and when not is a
> judgement call on behalf of the submitter.
> 
>>
>> If it is not based on Romain's patch, then Suggested-by would be much
>> more to the point - and something the maintainer (Stefan) could easily
>> edit when signing off, if there were nothing else to change.
>>
>> Andreas
> 
> Suggested-by implies there wasn't a patch, just a suggestion.

To me, it doesn't imply that.

> Maintainer can add any text on to the patch, that's also
> accepted practice.  I am merely saying it does not make sense to push
> back on contributors who are just trying to be nice.
> 
> Really there's no rule I can see that says you should not
> add random tags to the commit log, and I don't see what would
> making up such rules gain us either.
> 
> This is why I'm still responding to this thread really,
> you seem to make up new requirements that submitters need to
> meet, such things would have to get buy-in from more maintainers
> before we ask everyone to comply.

I feel you are turning my words around in my mouth. Let's agree that we
don't agree here.

* I never redefined the DCO. The DCO applies to a single Sob and by my
reading does not restrict having more than one Sob at all, and you
agreed that this is common practice.
* I did not reject the patch on that basis, I remarked it alongside
contential review. What the contributor makes of it depends on him, and
he hasn't spoken up so far, so you're not his lawyer either.
* People are free to invent new *-bys, but they shouldn't when there
exists one of different name and precise meaning, in particular Sob.
This is not against random *-by, it's specifically about
Based-on-patch-by which sounded redundant to Sob for me.
* We are having GSoC students around, and they shouldn't be picking up
bad examples just because no one else cares to comment on or fix commit
messages like I do. Linux also lets a lot of typos slip through, doesn't
mean we need to when we notice them.

CC'ing PMM, who has stated opinions on how to do these things too, so
it's unfair to pretend that it were just me making this up.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg