Hi, At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote:> The latest tree with Tim''s acks are at: > > git clone git://oss.oracle.com/git/mrathor/xen.git . > git checkout pvh.v10.acked-1This branch has my Reviewed-by: on 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related changes), which I don''t remember giving. Please be careful about that sort of thing. There have been a few instances in the past of patches that went in on someone else''s ack that seem to have sprouted mine (not from Mukesh, I should add, and AFAICT through misunderstanding rather than malice). In future I am going to revert such patches when I notice them. Cheers, Tim.
On Thu, 1 Aug 2013 11:21:33 +0100 Tim Deegan <tim@xen.org> wrote:> Hi, > > At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote: > > The latest tree with Tim''s acks are at: > > > > git clone git://oss.oracle.com/git/mrathor/xen.git . > > git checkout pvh.v10.acked-1 > > This branch has my Reviewed-by: on > 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related > changes), which I don''t remember giving. Please be careful about that > sort of thing. > > There have been a few instances in the past of patches that went in on > someone else''s ack that seem to have sprouted mine (not from Mukesh, I > should add, and AFAICT through misunderstanding rather than malice). > In future I am going to revert such patches when I notice them. > > Cheers, > > Tim.Ok, I misunderstood whey you said the code looked OK, I assumed it was an implicit ack, as I''ve seen here in the past. I''ll make a note, Tim doesn''t give implicit acks... :)... I''ll remove your ack. Mukesh
On Mon, Aug 5, 2013 at 10:18 PM, Mukesh Rathor <mukesh.rathor@oracle.com> wrote:> On Thu, 1 Aug 2013 11:21:33 +0100 > Tim Deegan <tim@xen.org> wrote: > >> Hi, >> >> At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote: >> > The latest tree with Tim''s acks are at: >> > >> > git clone git://oss.oracle.com/git/mrathor/xen.git . >> > git checkout pvh.v10.acked-1 >> >> This branch has my Reviewed-by: on >> 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related >> changes), which I don''t remember giving. Please be careful about that >> sort of thing. >> >> There have been a few instances in the past of patches that went in on >> someone else''s ack that seem to have sprouted mine (not from Mukesh, I >> should add, and AFAICT through misunderstanding rather than malice). >> In future I am going to revert such patches when I notice them. >> >> Cheers, >> >> Tim. > > > Ok, I misunderstood whey you said the code looked OK, I assumed it was > an implicit ack, as I''ve seen here in the past. I''ll make a note, Tim > doesn''t give implicit acks... :)... > > I''ll remove your ack.I''m pretty sure no one gives implicit Acks. Saying the code looks OK is just that -- it says the code looks OK, not that the person is OK with the code going in, and absolutely not everything that "Reviewed-by" means (which is a lot more than an Ack). -George
On Tue, 2013-08-06 at 11:00 +0100, George Dunlap wrote:> On Mon, Aug 5, 2013 at 10:18 PM, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > > On Thu, 1 Aug 2013 11:21:33 +0100 > > Tim Deegan <tim@xen.org> wrote: > > > >> Hi, > >> > >> At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote: > >> > The latest tree with Tim''s acks are at: > >> > > >> > git clone git://oss.oracle.com/git/mrathor/xen.git . > >> > git checkout pvh.v10.acked-1 > >> > >> This branch has my Reviewed-by: on > >> 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related > >> changes), which I don''t remember giving. Please be careful about that > >> sort of thing. > >> > >> There have been a few instances in the past of patches that went in on > >> someone else''s ack that seem to have sprouted mine (not from Mukesh, I > >> should add, and AFAICT through misunderstanding rather than malice). > >> In future I am going to revert such patches when I notice them. > >> > >> Cheers, > >> > >> Tim. > > > > > > Ok, I misunderstood whey you said the code looked OK, I assumed it was > > an implicit ack, as I''ve seen here in the past. I''ll make a note, Tim > > doesn''t give implicit acks... :)... > > > > I''ll remove your ack. > > I''m pretty sure no one gives implicit Acks. Saying the code looks OK > is just that -- it says the code looks OK, not that the person is OK > with the code going in, and absolutely not everything that > "Reviewed-by" means (which is a lot more than an Ack).There are some maintainers who say something informal but when asked "can I take that as an ack" say yes, which eventually leads to laziness on the part of submitters (and indeed committers) who get bored of asking the clarify question. It doesn''t seem to be happening as much as it once was though. I think people, and maintainers in particular, need to get in the habit of spelling the ack or review-by out explicitly, because ambiguity does lead to confusion here. Ian
Konrad Rzeszutek Wilk
2013-Aug-06 13:00 UTC
Re: Spurious Acks (was Re: PVH domU patches....)
On Tue, Aug 06, 2013 at 11:00:37AM +0100, George Dunlap wrote:> On Mon, Aug 5, 2013 at 10:18 PM, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > > On Thu, 1 Aug 2013 11:21:33 +0100 > > Tim Deegan <tim@xen.org> wrote: > > > >> Hi, > >> > >> At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote: > >> > The latest tree with Tim''s acks are at: > >> > > >> > git clone git://oss.oracle.com/git/mrathor/xen.git . > >> > git checkout pvh.v10.acked-1 > >> > >> This branch has my Reviewed-by: on > >> 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related > >> changes), which I don''t remember giving. Please be careful about that > >> sort of thing. > >> > >> There have been a few instances in the past of patches that went in on > >> someone else''s ack that seem to have sprouted mine (not from Mukesh, I > >> should add, and AFAICT through misunderstanding rather than malice). > >> In future I am going to revert such patches when I notice them. > >> > >> Cheers, > >> > >> Tim. > > > > > > Ok, I misunderstood whey you said the code looked OK, I assumed it was > > an implicit ack, as I''ve seen here in the past. I''ll make a note, Tim > > doesn''t give implicit acks... :)... > > > > I''ll remove your ack. > > I''m pretty sure no one gives implicit Acks. Saying the code looks OKI do. If I say ''code looks OK to me'' that implies to me ''Acked-by''. That is similar to how Linux works (from Documentation/SubmittingPatches): " Acked-by: is not as formal as Signed-off-by:. It is a record that the acker has at least reviewed the patch and has indicated acceptance. Hence patch mergers will sometimes manually convert an acker''s "yep, looks good to me" into an Acked-by:."> is just that -- it says the code looks OK, not that the person is OK > with the code going in, and absolutely not everything that > "Reviewed-by" means (which is a lot more than an Ack). > > -George > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
At 09:00 -0400 on 06 Aug (1375779658), Konrad Rzeszutek Wilk wrote:> On Tue, Aug 06, 2013 at 11:00:37AM +0100, George Dunlap wrote: > > On Mon, Aug 5, 2013 at 10:18 PM, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > > > On Thu, 1 Aug 2013 11:21:33 +0100 > > > Tim Deegan <tim@xen.org> wrote: > > > > > >> Hi, > > >> > > >> At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote: > > >> > The latest tree with Tim''s acks are at: > > >> > > > >> > git clone git://oss.oracle.com/git/mrathor/xen.git . > > >> > git checkout pvh.v10.acked-1 > > >> > > >> This branch has my Reviewed-by: on > > >> 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related > > >> changes), which I don''t remember giving. Please be careful about that > > >> sort of thing. > > >> > > >> There have been a few instances in the past of patches that went in on > > >> someone else''s ack that seem to have sprouted mine (not from Mukesh, I > > >> should add, and AFAICT through misunderstanding rather than malice). > > >> In future I am going to revert such patches when I notice them. > > >> > > >> Cheers, > > >> > > >> Tim. > > > > > > > > > Ok, I misunderstood whey you said the code looked OK, I assumed it was > > > an implicit ack, as I''ve seen here in the past. I''ll make a note, Tim > > > doesn''t give implicit acks... :)...That made me go back and look again -- what I said was: | If these aren''t already committed, you can add my Reviewed-by to all | except 9, 10, 19 and 23. [...] | #19 is probably OK for correctness, though I haven''t reviewed the VMCS | settings in enough detail to be sure they''re complete. My main | reservation is that it seems to duplicate a bunch of code from the | HVM VMCS setup. which doesn''t look to me anything like an implicit ack, much less a Reviewed-by -- in fact I explicitly called out #19 as _not_ being Reviewed-by. But yes, I don''t give implicit acks. And I''ll try to be clearer in future when I''m commenting on code that already has a relevant maintainer''s ack, as that seems to be where the confusion arises. I think I need a way of saying "I found no bugs in this patch but I still don''t like it". Cheers, Tim.
On Tue, Aug 06, 2013 at 09:00:58AM -0400, Konrad Rzeszutek Wilk wrote:> On Tue, Aug 06, 2013 at 11:00:37AM +0100, George Dunlap wrote: > > On Mon, Aug 5, 2013 at 10:18 PM, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > > > On Thu, 1 Aug 2013 11:21:33 +0100 > > > Tim Deegan <tim@xen.org> wrote: > > > > > >> Hi, > > >> > > >> At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote: > > >> > The latest tree with Tim''s acks are at: > > >> > > > >> > git clone git://oss.oracle.com/git/mrathor/xen.git . > > >> > git checkout pvh.v10.acked-1 > > >> > > >> This branch has my Reviewed-by: on > > >> 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related > > >> changes), which I don''t remember giving. Please be careful about that > > >> sort of thing. > > >> > > >> There have been a few instances in the past of patches that went in on > > >> someone else''s ack that seem to have sprouted mine (not from Mukesh, I > > >> should add, and AFAICT through misunderstanding rather than malice). > > >> In future I am going to revert such patches when I notice them. > > >> > > >> Cheers, > > >> > > >> Tim. > > > > > > > > > Ok, I misunderstood whey you said the code looked OK, I assumed it was > > > an implicit ack, as I''ve seen here in the past. I''ll make a note, Tim > > > doesn''t give implicit acks... :)... > > > > > > I''ll remove your ack. > > > > I''m pretty sure no one gives implicit Acks. Saying the code looks OK > > I do. If I say ''code looks OK to me'' that implies to me ''Acked-by''. > > That is similar to how Linux works (from Documentation/SubmittingPatches): > > " > Acked-by: is not as formal as Signed-off-by:. It is a record that the acker > has at least reviewed the patch and has indicated acceptance. Hence patch"at least reviewed the patch"? That''s news to me. Geroge once told me that Acked-by only means "I''m OK with this idea, I don''t even look at the patch at all". I''m quite confused here. :-( Wei.> mergers will sometimes manually convert an acker''s "yep, looks good to me" > into an Acked-by:." > > > > is just that -- it says the code looks OK, not that the person is OK > > with the code going in, and absolutely not everything that > > "Reviewed-by" means (which is a lot more than an Ack). > > > > -George > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Konrad Rzeszutek Wilk
2013-Aug-08 00:46 UTC
Re: Spurious Acks (was Re: PVH domU patches....)
On Tue, Aug 06, 2013 at 02:43:19PM +0100, Wei Liu wrote:> On Tue, Aug 06, 2013 at 09:00:58AM -0400, Konrad Rzeszutek Wilk wrote: > > On Tue, Aug 06, 2013 at 11:00:37AM +0100, George Dunlap wrote: > > > On Mon, Aug 5, 2013 at 10:18 PM, Mukesh Rathor <mukesh.rathor@oracle.com> wrote: > > > > On Thu, 1 Aug 2013 11:21:33 +0100 > > > > Tim Deegan <tim@xen.org> wrote: > > > > > > > >> Hi, > > > >> > > > >> At 14:43 -0700 on 31 Jul (1375281803), Mukesh Rathor wrote: > > > >> > The latest tree with Tim''s acks are at: > > > >> > > > > >> > git clone git://oss.oracle.com/git/mrathor/xen.git . > > > >> > git checkout pvh.v10.acked-1 > > > >> > > > >> This branch has my Reviewed-by: on > > > >> 1f2087845751569fc55c202ac3265e18c974b0bf (PVH xen: vmcs related > > > >> changes), which I don''t remember giving. Please be careful about that > > > >> sort of thing. > > > >> > > > >> There have been a few instances in the past of patches that went in on > > > >> someone else''s ack that seem to have sprouted mine (not from Mukesh, I > > > >> should add, and AFAICT through misunderstanding rather than malice). > > > >> In future I am going to revert such patches when I notice them. > > > >> > > > >> Cheers, > > > >> > > > >> Tim. > > > > > > > > > > > > Ok, I misunderstood whey you said the code looked OK, I assumed it was > > > > an implicit ack, as I''ve seen here in the past. I''ll make a note, Tim > > > > doesn''t give implicit acks... :)... > > > > > > > > I''ll remove your ack. > > > > > > I''m pretty sure no one gives implicit Acks. Saying the code looks OK > > > > I do. If I say ''code looks OK to me'' that implies to me ''Acked-by''. > > > > That is similar to how Linux works (from Documentation/SubmittingPatches): > > > > " > > Acked-by: is not as formal as Signed-off-by:. It is a record that the acker > > has at least reviewed the patch and has indicated acceptance. Hence patch > > "at least reviewed the patch"? That''s news to me. Geroge once told me > that Acked-by only means "I''m OK with this idea, I don''t even look at the > patch at all". I''m quite confused here. :-(Maybe we should copy the SubmittingPatches from Linux in the file so we have it in there and just base it on that?> > > Wei. > > > mergers will sometimes manually convert an acker''s "yep, looks good to me" > > into an Acked-by:." > > > > > > > is just that -- it says the code looks OK, not that the person is OK > > > with the code going in, and absolutely not everything that > > > "Reviewed-by" means (which is a lot more than an Ack). > > > > > > -George > > > > > > _______________________________________________ > > > Xen-devel mailing list > > > Xen-devel@lists.xen.org > > > http://lists.xen.org/xen-devel > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel
>>> On 08.08.13 at 02:46, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > Maybe we should copy the SubmittingPatches from Linux in the file so we > have it in there and just base it on that?Our flow is a bit different from theirs, so doing so without some editing might cause more confusion than clarification. Jan
Konrad Rzeszutek Wilk
2013-Aug-08 12:57 UTC
Re: Spurious Acks (was Re: PVH domU patches....)
On Thu, Aug 08, 2013 at 08:24:06AM +0100, Jan Beulich wrote:> >>> On 08.08.13 at 02:46, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > Maybe we should copy the SubmittingPatches from Linux in the file so we > > have it in there and just base it on that? > > Our flow is a bit different from theirs, so doing so without some > editing might cause more confusion than clarification.Sounds a like a perfect candidate then - otherwise we are bound to hit issues when folks are used to Linux try to develop in Xen and hit the process problem. I will post a patch with the initial copy of SubmittingPatches and then some on follow patches with some modifications of what I think had been discussed in the past.> > Jan >