Discussion:
[dpdk-dev] [PATCH] scripts: check cc stable mailing list in commit
(too old to reply)
Thomas Monjalon
2016-11-21 22:43:14 UTC
Permalink
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
They must be sent CC: ***@dpdk.org.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.

Signed-off-by: Thomas Monjalon <***@6wind.com>
---
scripts/check-git-log.sh | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index 5f8a9fc..4f98a7a 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -47,12 +47,14 @@ if [ "$1" = '-h' -o "$1" = '--help' ] ; then
exit
fi

+selfdir=$(dirname $(readlink -e $0))
range=${1:-origin/master..}

commits=$(git log --format='%h' --reverse $range)
headlines=$(git log --format='%s' --reverse $range)
bodylines=$(git log --format='%b' --reverse $range)
fixes=$(git log --format='%h %s' --reverse $range | grep -i ': *fix' | cut -d' ' -f1)
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 'fix.*:')
bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'

@@ -191,3 +193,10 @@ bad=$(for fixtag in $fixtags ; do
printf "$fixtag" | grep -v "^$good$"
done | sed 's,^,\t,')
[ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+ git log --format='%b' -1 $fix | grep -qi '^CC: ****@dpdk.org' ||
+ git log --format='\t%s' -1 $fix
+done)
+[ -z "$bad" ] || printf "Should CC: ***@dpdk.org\n$bad\n"
--
2.7.0
Ferruh Yigit
2016-11-30 14:54:14 UTC
Permalink
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
I think this is useful, thanks for the patch.
Post by Thomas Monjalon
---
scripts/check-git-log.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index 5f8a9fc..4f98a7a 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -47,12 +47,14 @@ if [ "$1" = '-h' -o "$1" = '--help' ] ; then
exit
fi
+selfdir=$(dirname $(readlink -e $0))
range=${1:-origin/master..}
commits=$(git log --format='%h' --reverse $range)
headlines=$(git log --format='%s' --reverse $range)
bodylines=$(git log --format='%b' --reverse $range)
fixes=$(git log --format='%h %s' --reverse $range | grep -i ': *fix' | cut -d' ' -f1)
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
range for git-log-fixes.sh.
Generates warning:
.../scripts/git-log-fixes.sh: illegal option -- 6
usage: git-log-fixes.sh [-h] <git_range>
Post by Thomas Monjalon
tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 'fix.*:')
bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'
@@ -191,3 +193,10 @@ bad=$(for fixtag in $fixtags ; do
printf "$fixtag" | grep -v "^$good$"
done | sed 's,^,\t,')
[ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+ git log --format='\t%s' -1 $fix
+done)
This is good for developer, but since "CC: xx" tags removed when patch
applied, this will generate warnings when run against existing history.

I don't know what can be done for this.

Or should we keep CC: tags in commit log perhaps?
Thomas Monjalon
2016-11-30 15:09:47 UTC
Permalink
Post by Ferruh Yigit
Post by Thomas Monjalon
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
range for git-log-fixes.sh.
.../scripts/git-log-fixes.sh: illegal option -- 6
usage: git-log-fixes.sh [-h] <git_range>
Yes, good catch.
I'm trying to fix it by converting -N to HEAD~N..

if printf -- $range | grep -q '^-[0-9]\+' ; then
range="HEAD$(printf -- $range | sed 's,^-,~,').."
fi
Post by Ferruh Yigit
Post by Thomas Monjalon
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+ git log --format='\t%s' -1 $fix
+done)
This is good for developer, but since "CC: xx" tags removed when patch
applied, this will generate warnings when run against existing history.
I do not think it is a problem.
Who runs this tool against existing history?
Post by Ferruh Yigit
I don't know what can be done for this.
Or should we keep CC: tags in commit log perhaps?
I do not see the value of keeping the CC: in the git history.
Bruce Richardson
2016-11-30 15:26:22 UTC
Permalink
Post by Thomas Monjalon
Post by Ferruh Yigit
Post by Thomas Monjalon
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
range for git-log-fixes.sh.
.../scripts/git-log-fixes.sh: illegal option -- 6
usage: git-log-fixes.sh [-h] <git_range>
Yes, good catch.
I'm trying to fix it by converting -N to HEAD~N..
if printf -- $range | grep -q '^-[0-9]\+' ; then
range="HEAD$(printf -- $range | sed 's,^-,~,').."
fi
Post by Ferruh Yigit
Post by Thomas Monjalon
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+ git log --format='\t%s' -1 $fix
+done)
This is good for developer, but since "CC: xx" tags removed when patch
applied, this will generate warnings when run against existing history.
I do not think it is a problem.
Who runs this tool against existing history?
Me for one. I prefer to run the script against the commits in the repo
before I generate the patches, rather than manually hand-editing the
patches afterward - or having to fix the repo and then regenerate them.
Also, when I was maintaining the next-net tree, I used to use pwclient git-am
to apply a patch, and then check-got-log.sh -1 to sanity check it once
build checks had passed.

/Bruce
Thomas Monjalon
2016-11-30 15:31:46 UTC
Permalink
Post by Bruce Richardson
Post by Thomas Monjalon
Post by Ferruh Yigit
Post by Thomas Monjalon
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
range for git-log-fixes.sh.
.../scripts/git-log-fixes.sh: illegal option -- 6
usage: git-log-fixes.sh [-h] <git_range>
Yes, good catch.
I'm trying to fix it by converting -N to HEAD~N..
if printf -- $range | grep -q '^-[0-9]\+' ; then
range="HEAD$(printf -- $range | sed 's,^-,~,').."
fi
Post by Ferruh Yigit
Post by Thomas Monjalon
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+ git log --format='\t%s' -1 $fix
+done)
This is good for developer, but since "CC: xx" tags removed when patch
applied, this will generate warnings when run against existing history.
I do not think it is a problem.
Who runs this tool against existing history?
Me for one. I prefer to run the script against the commits in the repo
before I generate the patches, rather than manually hand-editing the
patches afterward - or having to fix the repo and then regenerate them.
Also, when I was maintaining the next-net tree, I used to use pwclient git-am
to apply a patch, and then check-got-log.sh -1 to sanity check it once
build checks had passed.
I am not sure to understand.
You explain that you run the script for the commits you are going to send
or going to push. That's the normal usage.
In your cases you should have the CC: stable or you will have the warning.
Bruce Richardson
2016-11-30 15:36:07 UTC
Permalink
Post by Thomas Monjalon
Post by Bruce Richardson
Post by Thomas Monjalon
Post by Ferruh Yigit
Post by Thomas Monjalon
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
This breaks the "check-git-log.sh -N" usage, since "-N" is not a valid
range for git-log-fixes.sh.
.../scripts/git-log-fixes.sh: illegal option -- 6
usage: git-log-fixes.sh [-h] <git_range>
Yes, good catch.
I'm trying to fix it by converting -N to HEAD~N..
if printf -- $range | grep -q '^-[0-9]\+' ; then
range="HEAD$(printf -- $range | sed 's,^-,~,').."
fi
Post by Ferruh Yigit
Post by Thomas Monjalon
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+ git log --format='\t%s' -1 $fix
+done)
This is good for developer, but since "CC: xx" tags removed when patch
applied, this will generate warnings when run against existing history.
I do not think it is a problem.
Who runs this tool against existing history?
Me for one. I prefer to run the script against the commits in the repo
before I generate the patches, rather than manually hand-editing the
patches afterward - or having to fix the repo and then regenerate them.
Also, when I was maintaining the next-net tree, I used to use pwclient git-am
to apply a patch, and then check-got-log.sh -1 to sanity check it once
build checks had passed.
I am not sure to understand.
You explain that you run the script for the commits you are going to send
or going to push. That's the normal usage.
In your cases you should have the CC: stable or you will have the warning.
Ah, yes, good point.
Never mind.

/Bruce
Yuanhan Liu
2017-01-16 09:51:52 UTC
Permalink
Post by Ferruh Yigit
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
I think this is useful, thanks for the patch.
Yes, it is. Thanks! (Sorry for late reply; hope it's not too late).
Post by Ferruh Yigit
This is good for developer, but since "CC: xx" tags removed when patch
applied,
Again, I'd suggest to __not__ remove such tag. Firstly, why bother? And
I will talk why this tag should be kept, as a stable tree maintainer.

At the beginning, when people are not used to add "cc: stable" tag, I used
to pick bug fix commits from master by something like: list all bug fixing
patches and pick those that appliable to previous release.

Later, kudos to Thomas, who wrote an handy script (git-log-fixes.sh) to
do both, it indeeded make my life much easier. But it's still not enough.

It lists a lot of patches (206 fix patches, while 728 in total: the
ratio is near 30%):

$ devtools/git-log-fixes.sh v16.07..v16.11 | wc -l
206

$ git rev-list v16.07..v16.11 | wc -l
728

Thus I dropped few of them, manually, resulting to 130 (still looks like
a big number to me):

$ git rev-list v16.07..v16.07.2 | wc -l
130

The policy I would expect is, leave this tag as it is, I then will apply
all of them to a stable branch: I will no longer do the picking job. Instead,
I may just need handle those can't apply cleanly and ask the author to
do backport.

It would be do-able now, as I saw a lot of people are getting used to add
such tag. And even not, I saw those kind committers do that for them.

Besides, if there is already an explicit way, why should we stick on the
implicit way?

--yliu
Post by Ferruh Yigit
this will generate warnings when run against existing history.
I don't know what can be done for this.
Or should we keep CC: tags in commit log perhaps?
Thomas Monjalon
2017-01-16 10:37:34 UTC
Permalink
Post by Yuanhan Liu
Post by Ferruh Yigit
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
I think this is useful, thanks for the patch.
Yes, it is. Thanks! (Sorry for late reply; hope it's not too late).
Post by Ferruh Yigit
This is good for developer, but since "CC: xx" tags removed when patch
applied,
Again, I'd suggest to __not__ remove such tag. Firstly, why bother? And
I will talk why this tag should be kept, as a stable tree maintainer.
The drawback of keeping CC: in the mainline tree would be to give the wrong
feeling that only the commits having that tag are in the stable tree.
Post by Yuanhan Liu
At the beginning, when people are not used to add "cc: stable" tag, I used
to pick bug fix commits from master by something like: list all bug fixing
patches and pick those that appliable to previous release.
Later, kudos to Thomas, who wrote an handy script (git-log-fixes.sh) to
do both, it indeeded make my life much easier. But it's still not enough.
It lists a lot of patches (206 fix patches, while 728 in total: the
$ devtools/git-log-fixes.sh v16.07..v16.11 | wc -l
206
$ git rev-list v16.07..v16.11 | wc -l
728
Thus I dropped few of them, manually, resulting to 130 (still looks like
I do not think it is a so big number.
Post by Yuanhan Liu
$ git rev-list v16.07..v16.07.2 | wc -l
130
The policy I would expect is, leave this tag as it is, I then will apply
all of them to a stable branch: I will no longer do the picking job. Instead,
I may just need handle those can't apply cleanly and ask the author to
do backport.
It would be do-able now, as I saw a lot of people are getting used to add
such tag. And even not, I saw those kind committers do that for them.
The kind committers were adding CC: in their email reply.
You would like we add CC: also in the git tree before committing, right?

It put another responsibility on committers. When the stable tag is forgotten
by the author, it is easy to forget adding it in the commit. And when it is
pushed, it is too late. But we can still fix this neglecting by sending an
email to ***@dpdk.org.
That's why I think the most reliable source of explicit tagging is the stable
mailing list, not the mainline git tree.
Post by Yuanhan Liu
Besides, if there is already an explicit way, why should we stick on the
implicit way?
Because the explicit way is not 100% proof.
I think we still need to check manually to avoid missing too many fixes.
The other way (that you are advocating) means that we accept to miss
some fixes and it will enforce the community to become better and better to
manage that.
I can agree to your way of thinking. I just want to make it 100% clear to
everyone and read the feedbacks.

My conclusion: the work of stable maintainer should be simpler and better
automated. If we miss some fixes because of the automated process, it means
we must be better in this process :)
The most reliable source of trust in this automated process should be the
list of patches appearing on the stable mailing list at any time (on first
send or after it has been merged in the mainline).
Yuanhan Liu
2017-01-16 11:19:04 UTC
Permalink
Post by Thomas Monjalon
Post by Yuanhan Liu
Post by Ferruh Yigit
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
I think this is useful, thanks for the patch.
Yes, it is. Thanks! (Sorry for late reply; hope it's not too late).
Post by Ferruh Yigit
This is good for developer, but since "CC: xx" tags removed when patch
applied,
Again, I'd suggest to __not__ remove such tag. Firstly, why bother? And
I will talk why this tag should be kept, as a stable tree maintainer.
The drawback of keeping CC: in the mainline tree would be to give the wrong
feeling that only the commits having that tag are in the stable tree.
Idealy, if a patch should be in a stable tree, there should be a "cc: stable"
tag. And I don't think we are pretty far way from the idea case: as stated,
I already saw people are getting used to do that.

OTOH, why do we care how the commit log looks like in git history? Why do
why bother to ask "hey, why this commit log has this tag while that one
doesn't"?
Post by Thomas Monjalon
Post by Yuanhan Liu
The policy I would expect is, leave this tag as it is, I then will apply
all of them to a stable branch: I will no longer do the picking job. Instead,
I may just need handle those can't apply cleanly and ask the author to
do backport.
It would be do-able now, as I saw a lot of people are getting used to add
such tag. And even not, I saw those kind committers do that for them.
The kind committers were adding CC: in their email reply.
You would like we add CC: also in the git tree before committing, right?
Yes. I don't think that's too much effort, considering commiters normally
do some minors edits while apply, such as addding Reviewed|Acked|Xxx-by,
etc.
Post by Thomas Monjalon
It put another responsibility on committers. When the stable tag is forgotten
by the author, it is easy to forget adding it in the commit. And when it is
pushed, it is too late. But we can still fix this neglecting by sending an
Yes, that's why we have LTS release.
Post by Thomas Monjalon
That's why I think the most reliable source of explicit tagging is the stable
mailing list, not the mainline git tree.
There are too many patches there (in the stable mailing list), you never know
which are applied and which are not, especially when commiters are used to
change the commit subject a bit.
Post by Thomas Monjalon
Post by Yuanhan Liu
Besides, if there is already an explicit way, why should we stick on the
implicit way?
Because the explicit way is not 100% proof.
Yes, but I think it will be close to "100%" as time moves forward, when all
people are really used to add "cc: stable" tag, when there are just few need
to be added manually by the kind committers, when people know that the chance
your patch will not be in a stable release will be MUCH higher if you don't
do that.
Post by Thomas Monjalon
I think we still need to check manually to avoid missing too many fixes.
I agree. It's un-avoidable, especially in the first stage like where we are
now. But my purpose is to create some solid working styles, so that it would
be eaiser and convenient for everyone who wants to take such role in future.

That said, I will still look the git history, to do some extra manual check.
I may still use the script (git-log-fixes.sh) for a while. But the long goal
is to get rid of it, because there are so many things you have to check
mannually with that.
Post by Thomas Monjalon
The other way (that you are advocating) means that we accept to miss
some fixes and it will enforce the community to become better and better to
manage that.
I can agree to your way of thinking. I just want to make it 100% clear to
everyone and read the feedbacks.
My conclusion: the work of stable maintainer should be simpler and better
automated. If we miss some fixes because of the automated process, it means
we must be better in this process :)
The most reliable source of trust in this automated process should be the
list of patches appearing on the stable mailing list at any time (on first
send or after it has been merged in the mainline).
As stated, it's really hard: despite there are many versions, the biggest
difficult is commiters like to change the subject a bit. For example, here
is one work from myslef :/

[PATCH] vhost: Introduce vhost-user's REPLY_ACK feature

That's the one you see in the mailing list, and following is what you will
see in the git (after my twist):

vhost: introduce reply ack feature

--yliu
Thomas Monjalon
2017-01-16 14:26:03 UTC
Permalink
Post by Yuanhan Liu
Post by Thomas Monjalon
Post by Yuanhan Liu
Post by Ferruh Yigit
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
I think this is useful, thanks for the patch.
Yes, it is. Thanks! (Sorry for late reply; hope it's not too late).
Post by Ferruh Yigit
This is good for developer, but since "CC: xx" tags removed when patch
applied,
Again, I'd suggest to __not__ remove such tag. Firstly, why bother? And
I will talk why this tag should be kept, as a stable tree maintainer.
The drawback of keeping CC: in the mainline tree would be to give the wrong
feeling that only the commits having that tag are in the stable tree.
Idealy, if a patch should be in a stable tree, there should be a "cc: stable"
tag. And I don't think we are pretty far way from the idea case: as stated,
I already saw people are getting used to do that.
OTOH, why do we care how the commit log looks like in git history? Why do
why bother to ask "hey, why this commit log has this tag while that one
doesn't"?
Post by Thomas Monjalon
Post by Yuanhan Liu
The policy I would expect is, leave this tag as it is, I then will apply
all of them to a stable branch: I will no longer do the picking job. Instead,
I may just need handle those can't apply cleanly and ask the author to
do backport.
It would be do-able now, as I saw a lot of people are getting used to add
such tag. And even not, I saw those kind committers do that for them.
The kind committers were adding CC: in their email reply.
You would like we add CC: also in the git tree before committing, right?
Yes. I don't think that's too much effort, considering commiters normally
do some minors edits while apply, such as addding Reviewed|Acked|Xxx-by,
etc.
Post by Thomas Monjalon
It put another responsibility on committers. When the stable tag is forgotten
by the author, it is easy to forget adding it in the commit. And when it is
pushed, it is too late. But we can still fix this neglecting by sending an
Yes, that's why we have LTS release.
Post by Thomas Monjalon
That's why I think the most reliable source of explicit tagging is the stable
mailing list, not the mainline git tree.
There are too many patches there (in the stable mailing list), you never know
which are applied and which are not, especially when commiters are used to
change the commit subject a bit.
Post by Thomas Monjalon
Post by Yuanhan Liu
Besides, if there is already an explicit way, why should we stick on the
implicit way?
Because the explicit way is not 100% proof.
Yes, but I think it will be close to "100%" as time moves forward, when all
people are really used to add "cc: stable" tag, when there are just few need
to be added manually by the kind committers, when people know that the chance
your patch will not be in a stable release will be MUCH higher if you don't
do that.
Post by Thomas Monjalon
I think we still need to check manually to avoid missing too many fixes.
I agree. It's un-avoidable, especially in the first stage like where we are
now. But my purpose is to create some solid working styles, so that it would
be eaiser and convenient for everyone who wants to take such role in future.
That said, I will still look the git history, to do some extra manual check.
I may still use the script (git-log-fixes.sh) for a while. But the long goal
is to get rid of it, because there are so many things you have to check
mannually with that.
Post by Thomas Monjalon
The other way (that you are advocating) means that we accept to miss
some fixes and it will enforce the community to become better and better to
manage that.
I can agree to your way of thinking. I just want to make it 100% clear to
everyone and read the feedbacks.
My conclusion: the work of stable maintainer should be simpler and better
automated. If we miss some fixes because of the automated process, it means
we must be better in this process :)
The most reliable source of trust in this automated process should be the
list of patches appearing on the stable mailing list at any time (on first
send or after it has been merged in the mainline).
As stated, it's really hard: despite there are many versions, the biggest
difficult is commiters like to change the subject a bit. For example, here
is one work from myslef :/
[PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
That's the one you see in the mailing list, and following is what you will
vhost: introduce reply ack feature
OK I understand your long term goal.
I suggest to move forward by explaining this need in the contribution guide.
Yuanhan Liu
2017-01-16 14:46:09 UTC
Permalink
Post by Thomas Monjalon
Post by Yuanhan Liu
Post by Thomas Monjalon
Post by Yuanhan Liu
Besides, if there is already an explicit way, why should we stick on the
implicit way?
Because the explicit way is not 100% proof.
Yes, but I think it will be close to "100%" as time moves forward, when all
people are really used to add "cc: stable" tag, when there are just few need
to be added manually by the kind committers, when people know that the chance
your patch will not be in a stable release will be MUCH higher if you don't
do that.
Post by Thomas Monjalon
I think we still need to check manually to avoid missing too many fixes.
I agree. It's un-avoidable, especially in the first stage like where we are
now. But my purpose is to create some solid working styles, so that it would
be eaiser and convenient for everyone who wants to take such role in future.
That said, I will still look the git history, to do some extra manual check.
I may still use the script (git-log-fixes.sh) for a while. But the long goal
is to get rid of it, because there are so many things you have to check
mannually with that.
Post by Thomas Monjalon
The other way (that you are advocating) means that we accept to miss
some fixes and it will enforce the community to become better and better to
manage that.
I can agree to your way of thinking. I just want to make it 100% clear to
everyone and read the feedbacks.
My conclusion: the work of stable maintainer should be simpler and better
automated. If we miss some fixes because of the automated process, it means
we must be better in this process :)
The most reliable source of trust in this automated process should be the
list of patches appearing on the stable mailing list at any time (on first
send or after it has been merged in the mainline).
As stated, it's really hard: despite there are many versions, the biggest
difficult is commiters like to change the subject a bit. For example, here
is one work from myslef :/
[PATCH] vhost: Introduce vhost-user's REPLY_ACK feature
That's the one you see in the mailing list, and following is what you will
vhost: introduce reply ack feature
OK I understand your long term goal.
Thank you!
Post by Thomas Monjalon
I suggest to move forward by explaining this need in the contribution guide.
Sure. My plan was,

- make a contribution guide
- update the roadmap

The reason I have postoned the two for a while is I have few more thoughts
to refine the stable release a bit, which I will give more details in
another email in days.

--yliu

Ferruh Yigit
2017-01-16 10:38:02 UTC
Permalink
Post by Yuanhan Liu
Post by Ferruh Yigit
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
I think this is useful, thanks for the patch.
Yes, it is. Thanks! (Sorry for late reply; hope it's not too late).
Post by Ferruh Yigit
This is good for developer, but since "CC: xx" tags removed when patch
applied,
Again, I'd suggest to __not__ remove such tag. Firstly, why bother? And
I will talk why this tag should be kept, as a stable tree maintainer.
At the beginning, when people are not used to add "cc: stable" tag, I used
to pick bug fix commits from master by something like: list all bug fixing
patches and pick those that appliable to previous release.
Later, kudos to Thomas, who wrote an handy script (git-log-fixes.sh) to
do both, it indeeded make my life much easier. But it's still not enough.
It lists a lot of patches (206 fix patches, while 728 in total: the
$ devtools/git-log-fixes.sh v16.07..v16.11 | wc -l
206
$ git rev-list v16.07..v16.11 | wc -l
728
Thus I dropped few of them, manually, resulting to 130 (still looks like
$ git rev-list v16.07..v16.07.2 | wc -l
130
The policy I would expect is, leave this tag as it is, I then will apply
all of them to a stable branch: I will no longer do the picking job. Instead,
I may just need handle those can't apply cleanly and ask the author to
do backport.
Won't all patches that has CC:stable... also would have Fixes: line?
Post by Yuanhan Liu
It would be do-able now, as I saw a lot of people are getting used to add
such tag. And even not, I saw those kind committers do that for them.
Besides, if there is already an explicit way, why should we stick on the
implicit way?
--yliu
Post by Ferruh Yigit
this will generate warnings when run against existing history.
I don't know what can be done for this.
Or should we keep CC: tags in commit log perhaps?
Yuanhan Liu
2017-01-16 10:54:57 UTC
Permalink
Post by Ferruh Yigit
Post by Yuanhan Liu
Post by Ferruh Yigit
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
I think this is useful, thanks for the patch.
Yes, it is. Thanks! (Sorry for late reply; hope it's not too late).
Post by Ferruh Yigit
This is good for developer, but since "CC: xx" tags removed when patch
applied,
Again, I'd suggest to __not__ remove such tag. Firstly, why bother? And
I will talk why this tag should be kept, as a stable tree maintainer.
At the beginning, when people are not used to add "cc: stable" tag, I used
to pick bug fix commits from master by something like: list all bug fixing
patches and pick those that appliable to previous release.
Later, kudos to Thomas, who wrote an handy script (git-log-fixes.sh) to
do both, it indeeded make my life much easier. But it's still not enough.
It lists a lot of patches (206 fix patches, while 728 in total: the
$ devtools/git-log-fixes.sh v16.07..v16.11 | wc -l
206
$ git rev-list v16.07..v16.11 | wc -l
728
Thus I dropped few of them, manually, resulting to 130 (still looks like
$ git rev-list v16.07..v16.07.2 | wc -l
130
The policy I would expect is, leave this tag as it is, I then will apply
all of them to a stable branch: I will no longer do the picking job. Instead,
I may just need handle those can't apply cleanly and ask the author to
do backport.
Won't all patches that has CC:stable... also would have Fixes: line?
Yes, but it's not the reverse. And we now pick stable patches by the
fixline.

--yliu
Thomas Monjalon
2016-12-01 13:43:38 UTC
Permalink
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
They must be sent CC: ***@dpdk.org.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.

Signed-off-by: Thomas Monjalon <***@6wind.com>
---
v2: fix option -N
---
scripts/check-git-log.sh | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index 5f8a9fc..f79f0a2 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -47,12 +47,18 @@ if [ "$1" = '-h' -o "$1" = '--help' ] ; then
exit
fi

+selfdir=$(dirname $(readlink -e $0))
range=${1:-origin/master..}
+# convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
+if printf -- $range | grep -q '^-[0-9]\+' ; then
+ range="HEAD$(printf -- $range | sed 's,^-,~,').."
+fi

commits=$(git log --format='%h' --reverse $range)
headlines=$(git log --format='%s' --reverse $range)
bodylines=$(git log --format='%b' --reverse $range)
fixes=$(git log --format='%h %s' --reverse $range | grep -i ': *fix' | cut -d' ' -f1)
+stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d' ' -f2)
tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e 'fix.*:')
bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'

@@ -191,3 +197,10 @@ bad=$(for fixtag in $fixtags ; do
printf "$fixtag" | grep -v "^$good$"
done | sed 's,^,\t,')
[ -z "$bad" ] || printf "Wrong 'Fixes' reference:\n$bad\n"
+
+# check CC:stable for fixes
+bad=$(for fix in $stablefixes ; do
+ git log --format='%b' -1 $fix | grep -qi '^CC: ****@dpdk.org' ||
+ git log --format='\t%s' -1 $fix
+done)
+[ -z "$bad" ] || printf "Should CC: ***@dpdk.org\n$bad\n"
--
2.7.0
Ferruh Yigit
2016-12-01 15:00:32 UTC
Permalink
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
Tested-by: Ferruh Yigit <***@intel.com>
Thomas Monjalon
2016-12-01 15:03:43 UTC
Permalink
Post by Thomas Monjalon
Add a check for commits fixing a released bug.
Such commits are found thanks to scripts/git-log-fixes.sh.
In order to avoid forgetting CC, this mail header can be written
in the git commit message.
Applied
Continue reading on narkive:
Loading...