[dpdk-dev] [PATCH] devtools: check stable tag in fixes

[dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[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.

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>
> ---
<...>

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[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.

> 
> > 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.

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Thomas Monjalon]

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?"

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Yuanhan Liu]

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

[dpdk-dev] [PATCH v2] devtools: relax tag checking in fixes

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH v2] devtools: relax tag checking in fixes

[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!

	--yliu

Re: [dpdk-dev] [PATCH v2] devtools: relax tag checking in fixes

[Thomas Monjalon]

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

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Ferruh Yigit]

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

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Yuanhan Liu]

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

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Ferruh Yigit]

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
> 

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[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.
	--yliu

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Thomas Monjalon]

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.

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Yuanhan Liu]

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

Re: [dpdk-dev] [PATCH] devtools: check stable tag in fixes

[Ferruh Yigit]

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
>