Am 21.05.2014 14:11, schrieb Michael S. Tsirkin: > On Wed, May 21, 2014 at 12:42:13PM +0200, Andreas Färber wrote: >> 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: >>>>>>>> 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 <rom...@dolbeau.org> >>>>>>>> >>>>>>>> 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. >>> >>> 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. > > Hmm I certainly didn't intend to, so maybe there's a misunderstanding. > >> * 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. > > Ah OK. It's where you said "you should retain it" that made me think > you are saying tracking Sob from original sources is mandatory.
In case my use of "retain" was unclear, I specifically meant: Never *drop* Signed-off-bys from a patch you are modifying and resending. > so for the record, are you basically saying: > "you can replace Based-on-patch-by with Signed-off-by, it's > more or less equivalent, and more standard" > > > Then I agree. > Can you confirm please? Confirmed, assuming that original patch has such Signed-off-by(s). > I would add that > > > - it's possible to include text similar to > "Based on patch by ABC" in plain text > in addition to, or instead of the tag. Yep, when in doubt as explanation. Not machine-parseable of course, so my recommendation would be "in addition to", but otherwise equivalent to Some-newly-invented-by. > - If the patch is mostly identical to the original, with trivial > small modifications such as fixing typos, it's also possible to add > From: ABC > in front of the patch, this will then be the author recorded > in the project history Best, to not have it end up in the wrong place: git commit --amend --author="name <email>" Or actually the easiest variant: git am original-author-s.patch; git commit --amend -a -s > It's also possible to include the message id of the original patch if > you really want to, services such as mid.gmane.org can later be used to > look up messages based on the message id. To be precise, you probably meant a Message-id: line as added by Anthony's patches tool. e.g. http://git.qemu-project.org/?p=qemu.git;a=commit;h=e3da9921ebc554fad3224a9fdda9a7425ffd9ef7 Archive links OTOH can become stale after some months/years. But not wrong. And certainly useful in cover letter or, as here, below ---. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg