| Message ID | 20170531212056.10583-2-kpraveen.lkml@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 2017-06-01 at 02:50 +0530, Praveen Kumar wrote: > First, move some code around in order to make the next change more > obvious. > > commit 16c047add3ceaf0ab882e3e094d1ec904d02312d from linux tree > > Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com> > Mmm... actually, I am not absolutely sure of what's the formal process that we follow in this case is, but I think that: *) it is legally required (or at least preferable) that, at least the Signed-off-by, of the original authors of the patches are reported here; *) legally required or not, I think it's very useful to have them, and not only the S-o-b, but also the Acks, and all the other tags (like you had in the first posting of this series). You probably should add your own Signed-off-by, at the bottom of the list (this, you didn't do it in first posting of the series) I appreciate that you probably removed the tags to prevent git-send- email to use the enclosed email address and spam everyone, but I'm sure there is a way of telling git-send-email itself, to avoid doing that, even if you leave the tags and the emails in the patches' changelogs (something like a command line options, or similar.. I don't use it, so I can't tell). Regards, Dario
On 31/05/2017 23:40, Dario Faggioli wrote: > On Thu, 2017-06-01 at 02:50 +0530, Praveen Kumar wrote: >> First, move some code around in order to make the next change more >> obvious. >> >> commit 16c047add3ceaf0ab882e3e094d1ec904d02312d from linux tree >> >> Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com> >> > Mmm... actually, I am not absolutely sure of what's the formal process > that we follow in this case is, but I think that: > *) it is legally required (or at least preferable) that, at least the > Signed-off-by, of the original authors of the patches are reported > here; > *) legally required or not, I think it's very useful to have them, and > not only the S-o-b, but also the Acks, and all the other tags (like > you had in the first posting of this series). You probably should > add your own Signed-off-by, at the bottom of the list (this, you > didn't do it in first posting of the series) > > I appreciate that you probably removed the tags to prevent git-send- > email to use the enclosed email address and spam everyone, but I'm sure > there is a way of telling git-send-email itself, to avoid doing that, > even if you leave the tags and the emails in the patches' changelogs > (something like a command line options, or similar.. I don't use it, so > I can't tell). As an example, see http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=b01c2fb5834aea0328db55c310caa34173021d3d The way I prepare series like this for email is to use `git format-patch staging --cover-letter` to render the entire series as patch files in the local directory, edit each patch to put suitable Cc: lines beside the From: header, then `git send-email --dry-run *.patch --suppress-cc=all` to check what it will actually send. The Cc's in the header section are included, but no automatic Cc's are generated from content in the body. ~Andrew
On Wed, 2017-05-31 at 23:56 +0100, Andrew Cooper wrote: > As an example, see > > http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=b01c2fb5834ae > a0328db55c310caa34173021d3d > Nice, I especially like how the changelog looks, i.e.: - original Linux patch description description - Linux's Signed-off-by, Reviewed-by, Acked-by, etc. - ref to Linux commit id - a line with "Ported to Xen." - author of the port's Signed-off-by - (Reviewed-by, Acked-by, etc. coming from xen-devel) Praveen, I suggest using the same pattern (if you also like it, of course :-D). Using patch 1 as an example, that would mean the following: Subject: [PATCH 01/17] rb_tree: reorganize code in rb_erase() for additional changes First, move some code around in order to make the next change more obvious. [akpm@linux-foundation.org: coding-style fixes] Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> Signed-off-by: Wolfram Strepp <wstrepp@gmx.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> [Linux commit 16c047add3ceaf0ab882e3e094d1ec904d02312d] Ported to Xen. Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com> --- > The way I prepare series like this for email is to use `git format- > patch > staging --cover-letter` to render the entire series as patch files in > the local directory, edit each patch to put suitable Cc: lines > beside > the From: header, then `git send-email --dry-run *.patch > --suppress-cc=all` to check what it will actually send. The Cc's in > the > header section are included, but no automatic Cc's are generated from > content in the body. > Cool to know, thanks. Dario
On Thu, 2017-06-01 at 10:01 +0200, Dario Faggioli wrote: > On Wed, 2017-05-31 at 23:56 +0100, Andrew Cooper wrote: > > > > As an example, see > > > > http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=b01c2fb5834 > > ae > > a0328db55c310caa34173021d3d > > > Nice, I especially like how the changelog looks, i.e.: > - original Linux patch description description > - Linux's Signed-off-by, Reviewed-by, Acked-by, etc. > - ref to Linux commit id > - a line with "Ported to Xen." > - author of the port's Signed-off-by > - (Reviewed-by, Acked-by, etc. coming from xen-devel) > > Praveen, I suggest using the same pattern (if you also like it, of > course :-D). > Thanks Andrew for sharing the information. Yes, liked it too. Will incorporate the changes and share updated patch. Will try to do dry-run as suggested. > Using patch 1 as an example, that would mean the following: > > Subject: [PATCH 01/17] rb_tree: reorganize code in rb_erase() > for additional changes > > First, move some code around in order to make the next change more > obvious. > > [akpm@linux-foundation.org: coding-style fixes] > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl> > Signed-off-by: Wolfram Strepp <wstrepp@gmx.de> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> > [Linux commit 16c047add3ceaf0ab882e3e094d1ec904d02312d] > > Ported to Xen. > > Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com> > --- > > > > > The way I prepare series like this for email is to use `git format- > > patch > > staging --cover-letter` to render the entire series as patch files > > in > > the local directory, edit each patch to put suitable Cc: lines > > beside > > the From: header, then `git send-email --dry-run *.patch > > --suppress-cc=all` to check what it will actually send. The Cc's > > in > > the > > header section are included, but no automatic Cc's are generated > > from > > content in the body. > > > Cool to know, thanks. > Sure, I too was bit confused by replacing Signed-off-by. As suggested by Andrew, I will rework on the patches. Thanks again. Regards, ~Praveen.
>>> On 31.05.17 at 23:20, <kpraveen.lkml@gmail.com> wrote: > First, move some code around in order to make the next change more obvious. > > commit 16c047add3ceaf0ab882e3e094d1ec904d02312d from linux tree > > Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com> You've completely lost all original authorship - this is a no-go. You'll need to introduce a From: tag and retain at least the S-o-b-s of people having actively contributed to the change. Considering the original commit message, that's at least Peter and Andrew. If in doubt, keep all of them. You'd add yours below the ones you keep. Also please reproduce the full commit message (i.e. here also including Andrew's remark in square brackets). I guess the same will apply to the rest of the series, and since actually reviewing the content (when it comes from another, pre- reviewed source) is pretty pointless, I'll drop the entire series for now, in expectation of a v2. Jan
diff --git a/xen/common/rbtree.c b/xen/common/rbtree.c index 3328960d56..9826909a2a 100644 --- a/xen/common/rbtree.c +++ b/xen/common/rbtree.c @@ -236,6 +236,16 @@ void rb_erase(struct rb_node *node, struct rb_root *root) node = node->rb_right; while ((left = node->rb_left) != NULL) node = left; + + if (rb_parent(old)) + { + if (rb_parent(old)->rb_left == old) + rb_parent(old)->rb_left = node; + else + rb_parent(old)->rb_right = node; + } else + root->rb_node = node; + child = node->rb_right; parent = rb_parent(node); color = rb_color(node); @@ -252,15 +262,6 @@ void rb_erase(struct rb_node *node, struct rb_root *root) node->rb_right = old->rb_right; node->rb_left = old->rb_left; - if (rb_parent(old)) - { - if (rb_parent(old)->rb_left == old) - rb_parent(old)->rb_left = node; - else - rb_parent(old)->rb_right = node; - } else - root->rb_node = node; - rb_set_parent(old->rb_left, node); if (old->rb_right) rb_set_parent(old->rb_right, node);
First, move some code around in order to make the next change more obvious. commit 16c047add3ceaf0ab882e3e094d1ec904d02312d from linux tree Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com> --- xen/common/rbtree.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)