From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44262) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn5O2-00087G-7N for qemu-devel@nongnu.org; Wed, 21 May 2014 08:12:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn5Nu-0006uC-79 for qemu-devel@nongnu.org; Wed, 21 May 2014 08:12:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:3529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn5Nt-0006u0-VW for qemu-devel@nongnu.org; Wed, 21 May 2014 08:12:38 -0400 Date: Wed, 21 May 2014 15:11:25 +0300 From: "Michael S. Tsirkin" Message-ID: <20140521121125.GE19916@redhat.com> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <537C8305.9020406@suse.de> 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: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Peter Maydell , romain@dolbeau.org, qemu-devel@nongnu.org, agraf@suse.de, "Gabriel L. Somlo" , stefanha@redhat.com, pbonzini@redhat.com 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: > >>>>>> 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 > >>>>>> > >>>>>> 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. > >=20 > > It isn't useful if most of the patch has been changed, > > because Sob without the patch itself has no meaning. > >=20 > >> 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). > >=20 > > Saying "Based on patch" in commit log is common practice too. > >=20 > > 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: > >=20 > > (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 > >=20 > > 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. > >=20 > >> > >> 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. > >> > >> Andreas > >=20 > > Suggested-by implies there wasn't a patch, just a suggestion. >=20 > To me, it doesn't imply that. >=20 > > 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. > >=20 > > 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. > >=20 > > 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. >=20 > 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. 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? 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. - 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 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. > 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. Right. It's exactly for the benefit of the new people that I am responding here, we should try to make the rules explicit, to reduce the barrier of entry. > 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. >=20 > Andreas >=20 > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg