* [PATCH] devtools: check stable tag in fixes @ 2017-01-17 14:54 Thomas Monjalon 2017-01-17 18:15 ` Ferruh Yigit 2017-01-18 9:37 ` [PATCH v2] devtools: relax tag checking " Thomas Monjalon 0 siblings, 2 replies; 16+ messages in thread From: Thomas Monjalon @ 2017-01-17 14:54 UTC (permalink / raw) To: dev; +Cc: Yuanhan Liu The tag "Cc: stable@dpdk.org" must be set when the commit must be backported to a stable branch. It must be located just below the "Fixes:" tag (without blank line) and followed by a blank line, separated from SoB and review tags below. Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com> --- devtools/check-git-log.sh | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh index f6a35d2..9f1b435 100755 --- a/devtools/check-git-log.sh +++ b/devtools/check-git-log.sh @@ -170,9 +170,9 @@ bad=$(echo "$tags" | sed 's,^.,\t&,') [ -z "$bad" ] || printf "Wrong tag:\n$bad\n" -# check blank line after last Fixes: tag +# check blank line (or Cc: stable) after last Fixes: tag bad=$(echo "$bodylines" | - sed -n 'N;/\nFixes:/D;/\n$/D;/^Fixes:/P' | + sed -n 'N;/\nFixes:/D;/\nC[Cc]: stable@/D;/\n$/D;/^Fixes:/P' | sed 's,^.,\t&,') [ -z "$bad" ] || printf "Missing blank line after 'Fixes' tag:\n$bad\n" @@ -198,9 +198,15 @@ bad=$(for fixtag in $fixtags ; do done | sed 's,^,\t,') [ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n" -# check CC:stable for fixes +# check Cc: stable@dpdk.org for fixes bad=$(for fix in $stablefixes ; do - git log --format='%b' -1 $fix | grep -qi '^CC: *stable@dpdk.org' || + git log --format='%b' -1 $fix | grep -qi '^Cc: *stable@dpdk.org' || git log --format='\t%s' -1 $fix done) -[ -z "$bad" ] || printf "Should CC: stable@dpdk.org\n$bad\n" +[ -z "$bad" ] || printf "Should Cc: stable@dpdk.org\n$bad\n" + +# check blank line after Cc: stable@dpdk.org +bad=$(echo "$bodylines" | + sed -n 'N;/\n$/D;/^C[Cc]: stable@dpdk.org/P' | + sed 's,^.,\t&,') +[ -z "$bad" ] || printf "Missing blank line after 'Cc: stable@':\n$bad\n" -- 2.7.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-17 14:54 [PATCH] devtools: check stable tag in fixes Thomas Monjalon @ 2017-01-17 18:15 ` Ferruh Yigit 2017-01-17 18:42 ` Thomas Monjalon 2017-01-18 9:37 ` [PATCH v2] devtools: relax tag checking " Thomas Monjalon 1 sibling, 1 reply; 16+ messages in thread From: Ferruh Yigit @ 2017-01-17 18:15 UTC (permalink / raw) To: Thomas Monjalon, dev; +Cc: Yuanhan Liu On 1/17/2017 2:54 PM, Thomas Monjalon wrote: > The tag "Cc: stable@dpdk.org" must be set when the commit must be > backported to a stable branch. > > It must be located just below the "Fixes:" tag (without blank line) > and followed by a blank line, separated from SoB and review tags below. I am OK to keep it if it will help stable tree maintenance, but I still not clear about why we need this. If a patch is a fix, it should already have "Fixes:" line, so this can be used to parse git history. If patch is a feature, as far as I know still can go to stable tree, but for this case stable tree maintainer decides this, and author putting "Cc: stable@dpdk.org" tag not so useful. Author can put this tag just for recommendation, but if so why we are saving this into git history? Initially this was to be sure fixes CC'ed to stable mail list, now meaning is changing I guess. For the case author already cc'ed the stable tree but not put Cc: tag into git commit, should committer add this explicitly or ask from author a new version of the patch? Last thing, if this tag will remain in the commit log, is this only for stable tree, or any "Cc: <mail_address>" can stay in the history? > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com> > --- <...> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-17 18:15 ` Ferruh Yigit @ 2017-01-17 18:42 ` Thomas Monjalon 2017-01-18 4:41 ` Yuanhan Liu 0 siblings, 1 reply; 16+ messages in thread From: Thomas Monjalon @ 2017-01-17 18:42 UTC (permalink / raw) To: Ferruh Yigit, Yuanhan Liu; +Cc: dev, pablo.de.lara.guarch, bruce.richardson 2017-01-17 18:15, Ferruh Yigit: > On 1/17/2017 2:54 PM, Thomas Monjalon wrote: > > The tag "Cc: stable@dpdk.org" must be set when the commit must be > > backported to a stable branch. > > > > It must be located just below the "Fixes:" tag (without blank line) > > and followed by a blank line, separated from SoB and review tags below. > > I am OK to keep it if it will help stable tree maintenance, but I still > not clear about why we need this. > > If a patch is a fix, it should already have "Fixes:" line, so this can > be used to parse git history. That's a question for Yuanhan. My comments below: Some fixes are not candidate for the stable branch because the bug was not in the previous release. These fixes are filtered out by the script devtools/git-log-fixes.sh Some fixes are not so important and we can decide they do not fit in the stable branch. Who make this decision? Relying on this Cc tag would mean the committers decide which patch to backport. > If patch is a feature, as far as I know still can go to stable tree, but > for this case stable tree maintainer decides this, and author putting > "Cc: stable@dpdk.org" tag not so useful. Author can put this tag just > for recommendation, but if so why we are saving this into git history? No feature should be backported. > Initially this was to be sure fixes CC'ed to stable mail list, now > meaning is changing I guess. For the case author already cc'ed the > stable tree but not put Cc: tag into git commit, should committer add > this explicitly or ask from author a new version of the patch? Yuanhan was suggesting that the committer can do it if an author forget. > Last thing, if this tag will remain in the commit log, is this only for > stable tree, or any "Cc: <mail_address>" can stay in the history? I do not see the benefit of keeping other Cc in the history. It is just a convenience for authors when using git-send-email. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-17 18:42 ` Thomas Monjalon @ 2017-01-18 4:41 ` Yuanhan Liu 2017-01-18 8:32 ` Thomas Monjalon 2017-01-18 17:25 ` Ferruh Yigit 0 siblings, 2 replies; 16+ messages in thread From: Yuanhan Liu @ 2017-01-18 4:41 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Ferruh Yigit, dev, pablo.de.lara.guarch, bruce.richardson On Tue, Jan 17, 2017 at 07:42:33PM +0100, Thomas Monjalon wrote: > 2017-01-17 18:15, Ferruh Yigit: > > On 1/17/2017 2:54 PM, Thomas Monjalon wrote: > > > The tag "Cc: stable@dpdk.org" must be set when the commit must be > > > backported to a stable branch. > > > > > > It must be located just below the "Fixes:" tag (without blank line) > > > and followed by a blank line, separated from SoB and review tags below. > > > > I am OK to keep it if it will help stable tree maintenance, but I still > > not clear about why we need this. > > > > If a patch is a fix, it should already have "Fixes:" line, so this can > > be used to parse git history. Same answer (as I have already replied to you in another email): not all fix patches should be picked to stable branch. (I gave some examples below) > That's a question for Yuanhan. My comments below: > > Some fixes are not candidate for the stable branch because the bug was > not in the previous release. These fixes are filtered out by the script > devtools/git-log-fixes.sh > Some fixes are not so important and we can decide they do not fit in > the stable branch. Yes, I see no reason to pick patches that fix a typo, a comment issue, a coding style issue, or even, a hypothetic bug. Usually, it should be a patch that fixes a solid bug, like a crash, or something like "it behaves abnormally without the fix". That's the basic rule. Upon that, I think we could lower the bar a bit case by case. For example, I picked a doc update to v16.07.2: commit 92b70d21ee29bad92766699d0b45f579a2ff9adc Author: Jingjing Wu <jingjing.wu@intel.com> Date: Fri Sep 30 14:46:23 2016 +0800 doc: add limitations for i40e PMD [ upstream commit 972cc03ac4e30a7df8f734a77021bb15d0419b55 ] This patch adds "Limitations or Known issues" section for i40e PMD, including two items: 1. MPLS packet classification on X710/XL710 2. 16 Byte Descriptor cannot be used on DPDK VF 3. Link down with i40e kernel driver after DPDK application exist Signed-off-by: Jingjing Wu <jingjing.wu@intel.com> Acked-by: John McNamara <john.mcnamara@intel.com> It adds some limitations to the code introduced in last release: I think it's an important note the user might want to know if he sticks to that stable release. Talking about that, I think this patch makes abuse of the "Cc: stable" tag if a developer has to (or must) add such tag for a fix patch, no matter what it fixes. > Who make this decision? Relying on this Cc tag would > mean the committers decide which patch to backport. Ideally, it's the developer to make such decision: he knows the best what he is trying to fix, he also knows which version is vulnerable. But that's not something a new contributor might be aware of, and that's what the maintainer (not the committer) could be helpful here: tell him it's a candidate for a stable release and guide him on howto. The reason I want to stress the point of "the maintainer but not the committer" is, normally, the maintainer knows the code better. > > If patch is a feature, as far as I know still can go to stable tree, but > > for this case stable tree maintainer decides this, and author putting > > "Cc: stable@dpdk.org" tag not so useful. Author can put this tag just > > for recommendation, but if so why we are saving this into git history? > > No feature should be backported. > > > Initially this was to be sure fixes CC'ed to stable mail list, now > > meaning is changing I guess. For the case author already cc'ed the > > stable tree but not put Cc: tag into git commit, should committer add > > this explicitly or ask from author a new version of the patch? > > Yuanhan was suggesting that the committer can do it if an author forget. Yes, it's just part of the committer job, say, adding Tested-by, Reviewed-by, that kind of stuff. > > > Last thing, if this tag will remain in the commit log, is this only for > > stable tree, or any "Cc: <mail_address>" can stay in the history? > > I do not see the benefit of keeping other Cc in the history. Again, I really don't know why you bother to remove it, manually, commit by commit. What's the gain here? It just adds more burden to a committer. Honestly, I never do that. It actually has benefits. Keeping the cc tags allows us to cc them when we find a bug in that patch and make a fix patch later: they are likely to want to know any follow updates on that original patch. It also helps when I send out a stable patch review: I send a copy to all guys on the Cc list, on the Ack/Review, SoB list. I think they might also want to know that patch is a candidate for a specific branch. He may even give some comments, something like "if you pick this one, you might want to pick another one", or "why bother to port it to a stable release", that kind of thing. Honestly, for a commit log, besides the very basic format issues, like too long lines, words are tightly organized, I just care (and care a lot) one thing only: does the commit log states thing clearly? If it's a bug fix, does it state how the bug comes from and how to fix it, very specifically? If it's a feature patch (or set), does it describes what the feature is, what's it for, how it's implemented, with great details? If yes, I'm very happy about it. If no, I'd complain about it and ask him to reword it. After all, that's something I really want to know when I look back the git history (it normally happens when I want to know more explanations than the code could provide me). I don't care if it has few more Cc lines, few more fix lines, few more SoBs, etc. I don't even care it has typos, don't even to say he writes "rx" but not "Rx". Sorry, too much "complains" :) --yliu > It is just a convenience for authors when using git-send-email. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-18 4:41 ` Yuanhan Liu @ 2017-01-18 8:32 ` Thomas Monjalon 2017-01-18 8:54 ` Yuanhan Liu 2017-01-18 17:25 ` Ferruh Yigit 1 sibling, 1 reply; 16+ messages in thread From: Thomas Monjalon @ 2017-01-18 8:32 UTC (permalink / raw) To: Yuanhan Liu; +Cc: Ferruh Yigit, dev, pablo.de.lara.guarch, bruce.richardson 2017-01-18 12:41, Yuanhan Liu: > On Tue, Jan 17, 2017 at 07:42:33PM +0100, Thomas Monjalon wrote: > > 2017-01-17 18:15, Ferruh Yigit: > > > On 1/17/2017 2:54 PM, Thomas Monjalon wrote: > > > > The tag "Cc: stable@dpdk.org" must be set when the commit must be > > > > backported to a stable branch. > > > > > > > > It must be located just below the "Fixes:" tag (without blank line) > > > > and followed by a blank line, separated from SoB and review tags below. > > > > > > I am OK to keep it if it will help stable tree maintenance, but I still > > > not clear about why we need this. > > > > > > If a patch is a fix, it should already have "Fixes:" line, so this can > > > be used to parse git history. > > Same answer (as I have already replied to you in another email): not all fix > patches should be picked to stable branch. (I gave some examples below) > > > That's a question for Yuanhan. My comments below: > > > > Some fixes are not candidate for the stable branch because the bug was > > not in the previous release. These fixes are filtered out by the script > > devtools/git-log-fixes.sh > > Some fixes are not so important and we can decide they do not fit in > > the stable branch. > > Yes, I see no reason to pick patches that fix a typo, a comment issue, > a coding style issue, or even, a hypothetic bug. > > Usually, it should be a patch that fixes a solid bug, like a crash, or > something like "it behaves abnormally without the fix". That's the basic > rule. Upon that, I think we could lower the bar a bit case by case. For > example, I picked a doc update to v16.07.2: > > commit 92b70d21ee29bad92766699d0b45f579a2ff9adc > Author: Jingjing Wu <jingjing.wu@intel.com> > Date: Fri Sep 30 14:46:23 2016 +0800 > > doc: add limitations for i40e PMD > > [ upstream commit 972cc03ac4e30a7df8f734a77021bb15d0419b55 ] > > This patch adds "Limitations or Known issues" section for > i40e PMD, including two items: > 1. MPLS packet classification on X710/XL710 > 2. 16 Byte Descriptor cannot be used on DPDK VF > 3. Link down with i40e kernel driver after DPDK application exist > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com> > Acked-by: John McNamara <john.mcnamara@intel.com> > > It adds some limitations to the code introduced in last release: I think > it's an important note the user might want to know if he sticks to that > stable release. > > Talking about that, I think this patch makes abuse of the "Cc: stable" > tag if a developer has to (or must) add such tag for a fix patch, no > matter what it fixes. > > > Who make this decision? Relying on this Cc tag would > > mean the committers decide which patch to backport. > > Ideally, it's the developer to make such decision: he knows the best what > he is trying to fix, he also knows which version is vulnerable. > > But that's not something a new contributor might be aware of, and that's > what the maintainer (not the committer) could be helpful here: tell him > it's a candidate for a stable release and guide him on howto. > > The reason I want to stress the point of "the maintainer but not the > committer" is, normally, the maintainer knows the code better. > > > > If patch is a feature, as far as I know still can go to stable tree, but > > > for this case stable tree maintainer decides this, and author putting > > > "Cc: stable@dpdk.org" tag not so useful. Author can put this tag just > > > for recommendation, but if so why we are saving this into git history? > > > > No feature should be backported. > > > > > Initially this was to be sure fixes CC'ed to stable mail list, now > > > meaning is changing I guess. For the case author already cc'ed the > > > stable tree but not put Cc: tag into git commit, should committer add > > > this explicitly or ask from author a new version of the patch? > > > > Yuanhan was suggesting that the committer can do it if an author forget. > > Yes, it's just part of the committer job, say, adding Tested-by, > Reviewed-by, that kind of stuff. So it should be stressed in the contribution guide that it is the responsibility of the author and the maintainer to put this Cc: tag. > > > Last thing, if this tag will remain in the commit log, is this only for > > > stable tree, or any "Cc: <mail_address>" can stay in the history? > > > > I do not see the benefit of keeping other Cc in the history. > > Again, I really don't know why you bother to remove it, manually, commit > by commit. What's the gain here? It just adds more burden to a committer. > Honestly, I never do that. I had the personal feeling that if the Cc: person is not in SoB, Ack or Review tags, it means he's not interested in this patch. After thinking more, I was probably wrong. > It actually has benefits. Keeping the cc tags allows us to cc them when > we find a bug in that patch and make a fix patch later: they are likely > to want to know any follow updates on that original patch. > > It also helps when I send out a stable patch review: I send a copy to > all guys on the Cc list, on the Ack/Review, SoB list. I think they might > also want to know that patch is a candidate for a specific branch. He may > even give some comments, something like "if you pick this one, you might > want to pick another one", or "why bother to port it to a stable release", > that kind of thing. OK Yuanhan, thanks for the explanations. I will send a v2 to relax the constraint of blank line below Fixes: tag, and change the warning when Cc: tag is missing. Or remove the warning? What do you think of: "Reminder: is it a candidate for stable@dpdk.org backport?" ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-18 8:32 ` Thomas Monjalon @ 2017-01-18 8:54 ` Yuanhan Liu 0 siblings, 0 replies; 16+ messages in thread From: Yuanhan Liu @ 2017-01-18 8:54 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Ferruh Yigit, dev, pablo.de.lara.guarch, bruce.richardson On Wed, Jan 18, 2017 at 09:32:22AM +0100, Thomas Monjalon wrote: > > Yes, it's just part of the committer job, say, adding Tested-by, > > Reviewed-by, that kind of stuff. > > So it should be stressed in the contribution guide that it is the > responsibility of the author and the maintainer to put this Cc: tag. Yes, I will do that. > > > > Last thing, if this tag will remain in the commit log, is this only for > > > > stable tree, or any "Cc: <mail_address>" can stay in the history? > > > > > > I do not see the benefit of keeping other Cc in the history. > > > > Again, I really don't know why you bother to remove it, manually, commit > > by commit. What's the gain here? It just adds more burden to a committer. > > Honestly, I never do that. > > I had the personal feeling that if the Cc: person is not in SoB, Ack or > Review tags, it means he's not interested in this patch. > After thinking more, I was probably wrong. It's just not commonly used in DPDK, just like people here seldom use Reviewed-by while everyone just throw an Acked-by. > > It actually has benefits. Keeping the cc tags allows us to cc them when > > we find a bug in that patch and make a fix patch later: they are likely > > to want to know any follow updates on that original patch. > > > > It also helps when I send out a stable patch review: I send a copy to > > all guys on the Cc list, on the Ack/Review, SoB list. I think they might > > also want to know that patch is a candidate for a specific branch. He may > > even give some comments, something like "if you pick this one, you might > > want to pick another one", or "why bother to port it to a stable release", > > that kind of thing. > > OK Yuanhan, thanks for the explanations. Thanks for "listening"! :) > I will send a v2 to relax the constraint of blank line below Fixes: tag, > and change the warning when Cc: tag is missing. Or remove the warning? > What do you think of: "Reminder: is it a candidate for stable@dpdk.org backport?" Yes, I think such Reminder is enough. It might put him thinking more. Thanks. --yliu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-18 4:41 ` Yuanhan Liu 2017-01-18 8:32 ` Thomas Monjalon @ 2017-01-18 17:25 ` Ferruh Yigit 2017-01-19 8:05 ` Yuanhan Liu 1 sibling, 1 reply; 16+ messages in thread From: Ferruh Yigit @ 2017-01-18 17:25 UTC (permalink / raw) To: Yuanhan Liu, Thomas Monjalon; +Cc: dev, pablo.de.lara.guarch, bruce.richardson On 1/18/2017 4:41 AM, Yuanhan Liu wrote: > On Tue, Jan 17, 2017 at 07:42:33PM +0100, Thomas Monjalon wrote: >> 2017-01-17 18:15, Ferruh Yigit: >>> On 1/17/2017 2:54 PM, Thomas Monjalon wrote: >>>> The tag "Cc: stable@dpdk.org" must be set when the commit must be >>>> backported to a stable branch. >>>> >>>> It must be located just below the "Fixes:" tag (without blank line) >>>> and followed by a blank line, separated from SoB and review tags below. >>> >>> I am OK to keep it if it will help stable tree maintenance, but I still >>> not clear about why we need this. >>> >>> If a patch is a fix, it should already have "Fixes:" line, so this can >>> be used to parse git history. > > Same answer (as I have already replied to you in another email): not all fix > patches should be picked to stable branch. (I gave some examples below) I was thinking all fixes will have "Cc: stable" tag, to be sure all fixes sent to stable mail list, and you will be the picking for stable tree. Now you are suggesting some fixes may have "Fixes:" tag but not "Cc: stable" tag. So this means now author/maintainer/committer will be the picking patches for stable tree, and to show this decision, will add "Cc: stable" to the commit log. If author puts the "Cc: stable" tag, git send-email will ensure this patch will be sent to stable mail list too. But if author missed the "Cc: stable" tag, will it be enough for you if committer adds the tag into commit log? Or should patch sent to stable mail list too? What is the relation between "Cc: stable" tag been in commit log and patch been sent to stable mail list? Thanks, ferruh ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-18 17:25 ` Ferruh Yigit @ 2017-01-19 8:05 ` Yuanhan Liu 2017-01-19 12:00 ` Ferruh Yigit 0 siblings, 1 reply; 16+ messages in thread From: Yuanhan Liu @ 2017-01-19 8:05 UTC (permalink / raw) To: Ferruh Yigit; +Cc: Thomas Monjalon, dev, pablo.de.lara.guarch, bruce.richardson On Wed, Jan 18, 2017 at 05:25:10PM +0000, Ferruh Yigit wrote: > On 1/18/2017 4:41 AM, Yuanhan Liu wrote: > > On Tue, Jan 17, 2017 at 07:42:33PM +0100, Thomas Monjalon wrote: > >> 2017-01-17 18:15, Ferruh Yigit: > >>> On 1/17/2017 2:54 PM, Thomas Monjalon wrote: > >>>> The tag "Cc: stable@dpdk.org" must be set when the commit must be > >>>> backported to a stable branch. > >>>> > >>>> It must be located just below the "Fixes:" tag (without blank line) > >>>> and followed by a blank line, separated from SoB and review tags below. > >>> > >>> I am OK to keep it if it will help stable tree maintenance, but I still > >>> not clear about why we need this. > >>> > >>> If a patch is a fix, it should already have "Fixes:" line, so this can > >>> be used to parse git history. > > > > Same answer (as I have already replied to you in another email): not all fix > > patches should be picked to stable branch. (I gave some examples below) > > I was thinking all fixes will have "Cc: stable" tag, to be sure all > fixes sent to stable mail list, and you will be the picking for stable tree. > > Now you are suggesting some fixes may have "Fixes:" tag but not "Cc: > stable" tag. > > So this means now author/maintainer/committer will be the picking > patches for stable tree, and to show this decision, will add "Cc: > stable" to the commit log. > > If author puts the "Cc: stable" tag, git send-email will ensure this > patch will be sent to stable mail list too. > But if author missed the "Cc: stable" tag, will it be enough for you if > committer adds the tag into commit log? Or should patch sent to stable > mail list too? Yes, that'd be enough. I will write a short script to monitor the official DPDK git tree regularly (say weekly), to see whether there are any new candidates (those with 'cc: stable' tag) for stable releases or not. If so, I will pick them to stable. > > What is the relation between "Cc: stable" tag been in commit log and > patch been sent to stable mail list? Not too much. The stable mailing list provides a place for further discussion on stable patches: - A notification will be sent to the stable mailing list when a patch is chosen as a candidate for a stable release. People can shout out if he thinks some patches should not have been picked for a stable release. - people can also jump out to say, "hey, seems you forgot commit xxx, which is a good have for stable release". --yliu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-19 8:05 ` Yuanhan Liu @ 2017-01-19 12:00 ` Ferruh Yigit 2017-01-20 7:59 ` Yuanhan Liu 0 siblings, 1 reply; 16+ messages in thread From: Ferruh Yigit @ 2017-01-19 12:00 UTC (permalink / raw) To: Yuanhan Liu; +Cc: Thomas Monjalon, dev, pablo.de.lara.guarch, bruce.richardson On 1/19/2017 8:05 AM, Yuanhan Liu wrote: > On Wed, Jan 18, 2017 at 05:25:10PM +0000, Ferruh Yigit wrote: >> On 1/18/2017 4:41 AM, Yuanhan Liu wrote: >>> On Tue, Jan 17, 2017 at 07:42:33PM +0100, Thomas Monjalon wrote: >>>> 2017-01-17 18:15, Ferruh Yigit: >>>>> On 1/17/2017 2:54 PM, Thomas Monjalon wrote: >>>>>> The tag "Cc: stable@dpdk.org" must be set when the commit must be >>>>>> backported to a stable branch. >>>>>> >>>>>> It must be located just below the "Fixes:" tag (without blank line) >>>>>> and followed by a blank line, separated from SoB and review tags below. >>>>> >>>>> I am OK to keep it if it will help stable tree maintenance, but I still >>>>> not clear about why we need this. >>>>> >>>>> If a patch is a fix, it should already have "Fixes:" line, so this can >>>>> be used to parse git history. >>> >>> Same answer (as I have already replied to you in another email): not all fix >>> patches should be picked to stable branch. (I gave some examples below) >> >> I was thinking all fixes will have "Cc: stable" tag, to be sure all >> fixes sent to stable mail list, and you will be the picking for stable tree. >> >> Now you are suggesting some fixes may have "Fixes:" tag but not "Cc: >> stable" tag. >> >> So this means now author/maintainer/committer will be the picking >> patches for stable tree, and to show this decision, will add "Cc: >> stable" to the commit log. >> >> If author puts the "Cc: stable" tag, git send-email will ensure this >> patch will be sent to stable mail list too. >> But if author missed the "Cc: stable" tag, will it be enough for you if >> committer adds the tag into commit log? Or should patch sent to stable >> mail list too? > > Yes, that'd be enough. To highlight, and double check, the process will be updated as following: - Author may or may not have "CC: stable@dpdk.org" for fixes. - Maintainer/Committer may add "CC: stable@dpdk.org" tag to commit log, but won't send a mail to the stable mail list. - "CC: stable@dpdk.org" tags will remain in commit logs. > I will write a short script to monitor the > official DPDK git tree regularly (say weekly), to see whether there > are any new candidates (those with 'cc: stable' tag) for stable > releases or not. If so, I will pick them to stable. > >> >> What is the relation between "Cc: stable" tag been in commit log and >> patch been sent to stable mail list? > > Not too much. The stable mailing list provides a place for further > discussion on stable patches: > > - A notification will be sent to the stable mailing list when a patch > is chosen as a candidate for a stable release. People can shout out > if he thinks some patches should not have been picked for a stable > release. > > - people can also jump out to say, "hey, seems you forgot commit xxx, > which is a good have for stable release". > > --yliu > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-19 12:00 ` Ferruh Yigit @ 2017-01-20 7:59 ` Yuanhan Liu 2017-01-20 10:10 ` Thomas Monjalon 0 siblings, 1 reply; 16+ messages in thread From: Yuanhan Liu @ 2017-01-20 7:59 UTC (permalink / raw) To: Ferruh Yigit; +Cc: Thomas Monjalon, dev, pablo.de.lara.guarch, bruce.richardson On Thu, Jan 19, 2017 at 12:00:23PM +0000, Ferruh Yigit wrote: > To highlight, and double check, the process will be updated as following: > > - Author may or may not have "CC: stable@dpdk.org" for fixes. > > - Maintainer/Committer may add "CC: stable@dpdk.org" tag to commit log, > but won't send a mail to the stable mail list. > > - "CC: stable@dpdk.org" tags will remain in commit logs. Yes. --yliu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-20 7:59 ` Yuanhan Liu @ 2017-01-20 10:10 ` Thomas Monjalon 2017-01-20 10:23 ` Yuanhan Liu 0 siblings, 1 reply; 16+ messages in thread From: Thomas Monjalon @ 2017-01-20 10:10 UTC (permalink / raw) To: Yuanhan Liu, Ferruh Yigit; +Cc: dev, pablo.de.lara.guarch, bruce.richardson 2017-01-20 15:59, Yuanhan Liu: > On Thu, Jan 19, 2017 at 12:00:23PM +0000, Ferruh Yigit wrote: > > To highlight, and double check, the process will be updated as following: > > > > - Author may or may not have "CC: stable@dpdk.org" for fixes. > > > > - Maintainer/Committer may add "CC: stable@dpdk.org" tag to commit log, > > but won't send a mail to the stable mail list. > > > > - "CC: stable@dpdk.org" tags will remain in commit logs. > > Yes. Sorry, I do not understand why a maintainer or a committer would not send a mail to the stable ML. If we add the CC: tag, we can also CC:stable@dpdk.org in our reply. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-20 10:10 ` Thomas Monjalon @ 2017-01-20 10:23 ` Yuanhan Liu 2017-01-20 16:20 ` Ferruh Yigit 0 siblings, 1 reply; 16+ messages in thread From: Yuanhan Liu @ 2017-01-20 10:23 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Ferruh Yigit, dev, pablo.de.lara.guarch, bruce.richardson On Fri, Jan 20, 2017 at 11:10:54AM +0100, Thomas Monjalon wrote: > 2017-01-20 15:59, Yuanhan Liu: > > On Thu, Jan 19, 2017 at 12:00:23PM +0000, Ferruh Yigit wrote: > > > To highlight, and double check, the process will be updated as following: > > > > > > - Author may or may not have "CC: stable@dpdk.org" for fixes. > > > > > > - Maintainer/Committer may add "CC: stable@dpdk.org" tag to commit log, > > > but won't send a mail to the stable mail list. > > > > > > - "CC: stable@dpdk.org" tags will remain in commit logs. > > > > Yes. > > Sorry, I do not understand why a maintainer or a committer would not send a > mail to the stable ML. If we add the CC: tag, we can also CC:stable@dpdk.org > in our reply. Oh, right. --yliu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] devtools: check stable tag in fixes 2017-01-20 10:23 ` Yuanhan Liu @ 2017-01-20 16:20 ` Ferruh Yigit 0 siblings, 0 replies; 16+ messages in thread From: Ferruh Yigit @ 2017-01-20 16:20 UTC (permalink / raw) To: Yuanhan Liu, Thomas Monjalon; +Cc: dev, pablo.de.lara.guarch, bruce.richardson On 1/20/2017 10:23 AM, Yuanhan Liu wrote: > On Fri, Jan 20, 2017 at 11:10:54AM +0100, Thomas Monjalon wrote: >> 2017-01-20 15:59, Yuanhan Liu: >>> On Thu, Jan 19, 2017 at 12:00:23PM +0000, Ferruh Yigit wrote: >>>> To highlight, and double check, the process will be updated as following: >>>> >>>> - Author may or may not have "CC: stable@dpdk.org" for fixes. >>>> >>>> - Maintainer/Committer may add "CC: stable@dpdk.org" tag to commit log, >>>> but won't send a mail to the stable mail list. >>>> >>>> - "CC: stable@dpdk.org" tags will remain in commit logs. >>> >>> Yes. >> >> Sorry, I do not understand why a maintainer or a committer would not send a >> mail to the stable ML. If we add the CC: tag, we can also CC:stable@dpdk.org >> in our reply. > > Oh, right. Because if the decision will be given based on parsing git history, same patch being sent to stable mail list will be duplicate and will create noise. But if this will help somebody, somehow, I don't see any issue to sending mail to stable mail list. > > --yliu > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] devtools: relax tag checking in fixes 2017-01-17 14:54 [PATCH] devtools: check stable tag in fixes Thomas Monjalon 2017-01-17 18:15 ` Ferruh Yigit @ 2017-01-18 9:37 ` Thomas Monjalon 2017-01-18 10:04 ` Yuanhan Liu 1 sibling, 1 reply; 16+ messages in thread From: Thomas Monjalon @ 2017-01-18 9:37 UTC (permalink / raw) To: Yuanhan Liu; +Cc: dev The tag "Cc: stable@dpdk.org" must be set when the commit must be backported to a stable branch. The reminder is reworded. It should be located just below the "Fixes:" tag (without blank line) and followed by a blank line, separated from SoB and review tags below. However, there is no strong need for checking blank lines. Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com> --- v2: relax checking and reword reminder v1: strict checking of blank lines while allowing Cc: stable --- devtools/check-git-log.sh | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh index f6a35d2..62b5f43 100755 --- a/devtools/check-git-log.sh +++ b/devtools/check-git-log.sh @@ -170,12 +170,6 @@ bad=$(echo "$tags" | sed 's,^.,\t&,') [ -z "$bad" ] || printf "Wrong tag:\n$bad\n" -# check blank line after last Fixes: tag -bad=$(echo "$bodylines" | - sed -n 'N;/\nFixes:/D;/\n$/D;/^Fixes:/P' | - sed 's,^.,\t&,') -[ -z "$bad" ] || printf "Missing blank line after 'Fixes' tag:\n$bad\n" - # check missing Fixes: tag bad=$(for fix in $fixes ; do git log --format='%b' -1 $fix | grep -q '^Fixes: ' || @@ -198,9 +192,9 @@ bad=$(for fixtag in $fixtags ; do done | sed 's,^,\t,') [ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n" -# check CC:stable for fixes +# check Cc: stable@dpdk.org for fixes bad=$(for fix in $stablefixes ; do - git log --format='%b' -1 $fix | grep -qi '^CC: *stable@dpdk.org' || + git log --format='%b' -1 $fix | grep -qi '^Cc: *stable@dpdk.org' || git log --format='\t%s' -1 $fix done) -[ -z "$bad" ] || printf "Should CC: stable@dpdk.org\n$bad\n" +[ -z "$bad" ] || printf "Is it candidate for Cc: stable@dpdk.org backport?\n$bad\n" -- 2.7.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] devtools: relax tag checking in fixes 2017-01-18 9:37 ` [PATCH v2] devtools: relax tag checking " Thomas Monjalon @ 2017-01-18 10:04 ` Yuanhan Liu 2017-01-18 15:52 ` Thomas Monjalon 0 siblings, 1 reply; 16+ messages in thread From: Yuanhan Liu @ 2017-01-18 10:04 UTC (permalink / raw) To: Thomas Monjalon; +Cc: dev On Wed, Jan 18, 2017 at 10:37:52AM +0100, Thomas Monjalon wrote: > The tag "Cc: stable@dpdk.org" must be set when the commit must be > backported to a stable branch. The reminder is reworded. > > It should be located just below the "Fixes:" tag (without blank line) > and followed by a blank line, separated from SoB and review tags below. > However, there is no strong need for checking blank lines. > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com> Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> Thanks! --yliu ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] devtools: relax tag checking in fixes 2017-01-18 10:04 ` Yuanhan Liu @ 2017-01-18 15:52 ` Thomas Monjalon 0 siblings, 0 replies; 16+ messages in thread From: Thomas Monjalon @ 2017-01-18 15:52 UTC (permalink / raw) To: Yuanhan Liu; +Cc: dev 2017-01-18 18:04, Yuanhan Liu: > On Wed, Jan 18, 2017 at 10:37:52AM +0100, Thomas Monjalon wrote: > > The tag "Cc: stable@dpdk.org" must be set when the commit must be > > backported to a stable branch. The reminder is reworded. > > > > It should be located just below the "Fixes:" tag (without blank line) > > and followed by a blank line, separated from SoB and review tags below. > > However, there is no strong need for checking blank lines. > > > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com> > > Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com> > > Thanks! Applied ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-01-20 16:20 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-17 14:54 [PATCH] devtools: check stable tag in fixes Thomas Monjalon 2017-01-17 18:15 ` Ferruh Yigit 2017-01-17 18:42 ` Thomas Monjalon 2017-01-18 4:41 ` Yuanhan Liu 2017-01-18 8:32 ` Thomas Monjalon 2017-01-18 8:54 ` Yuanhan Liu 2017-01-18 17:25 ` Ferruh Yigit 2017-01-19 8:05 ` Yuanhan Liu 2017-01-19 12:00 ` Ferruh Yigit 2017-01-20 7:59 ` Yuanhan Liu 2017-01-20 10:10 ` Thomas Monjalon 2017-01-20 10:23 ` Yuanhan Liu 2017-01-20 16:20 ` Ferruh Yigit 2017-01-18 9:37 ` [PATCH v2] devtools: relax tag checking " Thomas Monjalon 2017-01-18 10:04 ` Yuanhan Liu 2017-01-18 15:52 ` Thomas Monjalon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.