From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47759) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn8yx-0004oN-Ok for qemu-devel@nongnu.org; Wed, 21 May 2014 12:03:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn8ys-0001XD-PY for qemu-devel@nongnu.org; Wed, 21 May 2014 12:03:07 -0400 Received: from cantor2.suse.de ([195.135.220.15]:33200 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn8ys-0001WH-Gn for qemu-devel@nongnu.org; Wed, 21 May 2014 12:03:02 -0400 Message-ID: <537CCE2D.1070608@suse.de> Date: Wed, 21 May 2014 18:02:53 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <20140520150510.GH1657@ERROL.INI.CMU.EDU> <537C6696.9000509@suse.de> <20140521090456.GC19109@redhat.com> <537C6E0A.4020200@suse.de> <20140521092545.GA19646@redhat.com> <537C7680.4000000@suse.de> <20140521102241.GC19916@redhat.com> <537C8305.9020406@suse.de> <20140521121125.GE19916@redhat.com> In-Reply-To: <20140521121125.GE19916@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] e1000: allow command-line selection of card model List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Peter Maydell , romain@dolbeau.org, qemu-devel@nongnu.org, agraf@suse.de, "Gabriel L. Somlo" , stefanha@redhat.com, pbonzini@redhat.com Am 21.05.2014 14:11, schrieb Michael S. Tsirkin: > On Wed, May 21, 2014 at 12:42:13PM +0200, Andreas F=E4rber wrote: >> Am 21.05.2014 12:22, schrieb Michael S. Tsirkin: >>> On Wed, May 21, 2014 at 11:48:48AM +0200, Andreas F=E4rber wrote: >>>> Am 21.05.2014 11:25, schrieb Michael S. Tsirkin: >>>>> On Wed, May 21, 2014 at 11:12:42AM +0200, Andreas F=E4rber wrote: >>>>>> Am 21.05.2014 11:04, schrieb Michael S. Tsirkin: >>>>>>> On Wed, May 21, 2014 at 10:40:54AM +0200, Andreas F=E4rber 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 >>>>>>>> >>>>>>>> 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 Origi= n 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 X= YZ". >>>>> >>>>> >>>>>> 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 Dolbea= u, 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 t= hen >>>>> 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 ba= sed >>>> 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, dependi= ng >>>> 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 sou= rce >>> license and I have the right under that license to submit= that >>> work with modifications, whether created in whole or in p= art >>> by me, under the same open source license (unless I am >>> permitted to submit under a different license), as indica= ted >>> 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 ma= ke >>> 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 muc= h >>>> more to the point - and something the maintainer (Stefan) could easi= ly >>>> 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 w= e >> don't agree here. >=20 > Hmm I certainly didn't intend to, so maybe there's a misunderstanding. >=20 >> * 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. >=20 > 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" >=20 >=20 > Then I agree. > Can you confirm please? Confirmed, assuming that original patch has such Signed-off-by(s). > I would add that >=20 >=20 > - 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=3D"name " 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=3Dqemu.git;a=3Dcommit;h=3De3da9921ebc554fa= d3224a9fdda9a7425ffd9ef7 Archive links OTOH can become stale after some months/years. But not wrong. And certainly useful in cover letter or, as here, below ---. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg