[dpdk-dev] [PATCH 0/3] more commits checks

[dpdk-dev] [PATCH 0/3] more commits checks

[Thomas Monjalon]

The script check-git-log.sh helps to keep a consistent git history.
Scrolling/parsing the history is easier when we are used to some
rules.

Nélio Laranjeiro (1):
  scripts: check headline of drivers commits

Thomas Monjalon (2):
  scripts: check spacing after commit references
  scripts: reduce line size of commit checks

 scripts/check-git-log.sh | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

-- 
2.7.0

[dpdk-dev] [PATCH 1/3] scripts: check spacing after commit references

[Thomas Monjalon]

A blank line between "Fixes" and "Signed-off-by" helps to
separate information in blocks.

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

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index c7fc0e4..a24f101 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -129,6 +129,12 @@ 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: ' ||
-- 
2.7.0

[dpdk-dev] [PATCH 2/3] scripts: reduce line size of commit checks

[Thomas Monjalon]

Small cleanup to comply with the 80 chars limit.

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

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index a24f101..833aa39 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -52,8 +52,9 @@ range=${1:-origin/master..}
 commits=$(git log --format='%h' $range)
 headlines=$(git log --format='%s' $range)
 bodylines=$(git log --format='%b' $range)
-tags=$(git log --format='%b' $range | grep -i -e 'by *:' -e 'fix.*:')
 fixes=$(git log --format='%h %s' $range | grep -i ': *fix' | cut -d' ' -f1)
+tags=$(git log --format='%b' $range | grep -i -e 'by *:' -e 'fix.*:')
+bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by:'
 
 # check headline format (spacing, no punctuation, no code)
 bad=$(echo "$headlines" | grep --color=always \
@@ -108,11 +109,15 @@ bad=$(echo "$headlines" | grep -E --color=always \
 [ -z "$bad" ] || printf "Wrong headline lowercase:\n$bad\n"
 
 # check headline length (60 max)
-bad=$(echo "$headlines" | awk 'length>60 {print}' | sed 's,^,\t,')
+bad=$(echo "$headlines" |
+	awk 'length>60 {print}' |
+	sed 's,^,\t,')
 [ -z "$bad" ] || printf "Headline too long:\n$bad\n"
 
 # check body lines length (75 max)
-bad=$(echo "$bodylines" | grep -v '^Fixes:' | awk 'length>75 {print}' | sed 's,^,\t,')
+bad=$(echo "$bodylines" | grep -v '^Fixes:' |
+	awk 'length>75 {print}' |
+	sed 's,^,\t,')
 [ -z "$bad" ] || printf "Line too long:\n$bad\n"
 
 # check starting commit message with "It"
@@ -124,7 +129,7 @@ done | sed 's,^,\t,')
 
 # check tags spelling
 bad=$(echo "$tags" |
-	grep -v '^\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-by: [^,]* <.*@.*>$' |
+	grep -v "^$bytag [^,]* <.*@.*>$" |
 	grep -v '^Fixes: [0-9a-f]\{7\}[0-9a-f]* (".*")$' |
 	sed 's,^.,\t&,')
 [ -z "$bad" ] || printf "Wrong tag:\n$bad\n"
-- 
2.7.0

[dpdk-dev] [PATCH 3/3] scripts: check headline of drivers commits

[Thomas Monjalon]

From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

A driver patch under net should start with "net/<driver name>" or if
a patch touch multiple drivers, it should only start with "net:".
The same apply for crypto.
A patch touching all drivers (net + crypto) should start with "drivers:".

Longer prefixes like "net/mlx:" (for mlx4/mlx5) or "net/e1000/base:" are
handled by not checking the colon.

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 scripts/check-git-log.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index 833aa39..3f3cf19 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -70,6 +70,23 @@ bad=$(echo "$headlines" | grep --color=always \
 	| sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
 
+# check headline prefix when touching only drivers/, e.g. net/<driver name>
+bad=$(for commit in $commits ; do
+	headline=$(git log --format='%s' -1 $commit)
+	files=$(git diff-tree --no-commit-id --name-only -r $commit)
+	[ -z "$(echo "$files" | grep -v '^drivers/')" ] || continue
+	driversgrp=$(echo "$files" | cut -d "/" -f 2 | sort -u)
+	drivers=$(echo "$files" | cut -d "/" -f 2,3 | sort -u)
+	if [ $(echo "$driversgrp" | wc -l) -gt 1 ] ; then
+		bad=$(echo "$headline" | grep -v '^drivers:')
+	elif [ $(echo "$drivers" | wc -l) -gt 1 ] ; then
+		bad=$(echo "$headline" | grep -v "^$driversgrp")
+	else
+		bad=$(echo "$headline" | grep -v "^$drivers")
+	fi
+done | sed 's,^,\t,')
+[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
+
 # check headline label for common typos
 bad=$(echo "$headlines" | grep --color=always \
 	-e '^example[:/]' \
-- 
2.7.0

Re: [dpdk-dev] [PATCH 3/3] scripts: check headline of drivers commits

[Bruce Richardson]

On Wed, Jul 06, 2016 at 12:38:50PM +0200, Thomas Monjalon wrote:
> From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> A driver patch under net should start with "net/<driver name>" or if
> a patch touch multiple drivers, it should only start with "net:".
> The same apply for crypto.
> A patch touching all drivers (net + crypto) should start with "drivers:".
> 
> Longer prefixes like "net/mlx:" (for mlx4/mlx5) or "net/e1000/base:" are
> handled by not checking the colon.
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  scripts/check-git-log.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
> index 833aa39..3f3cf19 100755
> --- a/scripts/check-git-log.sh
> +++ b/scripts/check-git-log.sh
> @@ -70,6 +70,23 @@ bad=$(echo "$headlines" | grep --color=always \
>  	| sed 's,^,\t,')
>  [ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
>  
> +# check headline prefix when touching only drivers/, e.g. net/<driver name>
> +bad=$(for commit in $commits ; do
> +	headline=$(git log --format='%s' -1 $commit)
> +	files=$(git diff-tree --no-commit-id --name-only -r $commit)
> +	[ -z "$(echo "$files" | grep -v '^drivers/')" ] || continue

Given the preference for including documentation in with the code changes,
we should perhaps exclude any doc changes when making this check.

/Bruce

Re: [dpdk-dev] [PATCH 3/3] scripts: check headline of drivers commits

[Thomas Monjalon]

2016-07-06 12:09, Bruce Richardson:
> On Wed, Jul 06, 2016 at 12:38:50PM +0200, Thomas Monjalon wrote:
> > From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > +# check headline prefix when touching only drivers/, e.g. net/<driver name>
> > +bad=$(for commit in $commits ; do
> > +	headline=$(git log --format='%s' -1 $commit)
> > +	files=$(git diff-tree --no-commit-id --name-only -r $commit)
> > +	[ -z "$(echo "$files" | grep -v '^drivers/')" ] || continue
> 
> Given the preference for including documentation in with the code changes,
> we should perhaps exclude any doc changes when making this check.

Yes we can exclude doc/ and config/.
Thanks

Re: [dpdk-dev] [PATCH 3/3] scripts: check headline of drivers commits

[Nélio Laranjeiro]

Thomas,

On Wed, Jul 06, 2016 at 12:38:50PM +0200, Thomas Monjalon wrote:
> From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> A driver patch under net should start with "net/<driver name>" or if
> a patch touch multiple drivers, it should only start with "net:".
> The same apply for crypto.
> A patch touching all drivers (net + crypto) should start with "drivers:".
> 
> Longer prefixes like "net/mlx:" (for mlx4/mlx5) or "net/e1000/base:" are
> handled by not checking the colon.
> 
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>  scripts/check-git-log.sh | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
> index 833aa39..3f3cf19 100755
> --- a/scripts/check-git-log.sh
> +++ b/scripts/check-git-log.sh
> @@ -70,6 +70,23 @@ bad=$(echo "$headlines" | grep --color=always \
>  	| sed 's,^,\t,')
>  [ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
>  
> +# check headline prefix when touching only drivers/, e.g. net/<driver name>
> +bad=$(for commit in $commits ; do
> +	headline=$(git log --format='%s' -1 $commit)
> +	files=$(git diff-tree --no-commit-id --name-only -r $commit)
> +	[ -z "$(echo "$files" | grep -v '^drivers/')" ] || continue
> +	driversgrp=$(echo "$files" | cut -d "/" -f 2 | sort -u)
> +	drivers=$(echo "$files" | cut -d "/" -f 2,3 | sort -u)
> +	if [ $(echo "$driversgrp" | wc -l) -gt 1 ] ; then
> +		bad=$(echo "$headline" | grep -v '^drivers:')

I think you forgot the '$' for drivers in the line just above.

> +	elif [ $(echo "$drivers" | wc -l) -gt 1 ] ; then
> +		bad=$(echo "$headline" | grep -v "^$driversgrp")
> +	else
> +		bad=$(echo "$headline" | grep -v "^$drivers")
> +	fi
> +done | sed 's,^,\t,')
> +[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
> +
>  # check headline label for common typos
>  bad=$(echo "$headlines" | grep --color=always \
>  	-e '^example[:/]' \
> -- 
> 2.7.0

Otherwise it seems good.

Regards,

-- 
Nélio Laranjeiro
6WIND

Re: [dpdk-dev] [PATCH 3/3] scripts: check headline of drivers commits

[Thomas Monjalon]

2016-07-06 13:59, Nélio Laranjeiro:
> > +# check headline prefix when touching only drivers/, e.g. net/<driver name>
> > +bad=$(for commit in $commits ; do
> > +	headline=$(git log --format='%s' -1 $commit)
> > +	files=$(git diff-tree --no-commit-id --name-only -r $commit)
> > +	[ -z "$(echo "$files" | grep -v '^drivers/')" ] || continue
> > +	driversgrp=$(echo "$files" | cut -d "/" -f 2 | sort -u)
> > +	drivers=$(echo "$files" | cut -d "/" -f 2,3 | sort -u)
> > +	if [ $(echo "$driversgrp" | wc -l) -gt 1 ] ; then
> > +		bad=$(echo "$headline" | grep -v '^drivers:')
> 
> I think you forgot the '$' for drivers in the line just above.

No it is a plain "drivers:" for cases where we have net and crypto.
(it is an addition from your original idea)
But I forgot to remove bad=$( which I've moved to the first line.

> > +	elif [ $(echo "$drivers" | wc -l) -gt 1 ] ; then
> > +		bad=$(echo "$headline" | grep -v "^$driversgrp")
> > +	else
> > +		bad=$(echo "$headline" | grep -v "^$drivers")
> > +	fi
> > +done | sed 's,^,\t,')
> > +[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
> 
> Otherwise it seems good.

V2 on going

[dpdk-dev] [PATCH v2] scripts: check headline of drivers commits

[Thomas Monjalon]

From: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

A driver patch under net should start with "net/<driver name>" or if
a patch touch multiple drivers, it should only start with "net:".
The same apply for crypto.
A patch touching all drivers (net + crypto) should start with "drivers:".

Longer prefixes like "net/mlx:" (for mlx4/mlx5) or "net/e1000/base:" are
handled by not checking the colon.

The directories doc/ and config/ are ignored because a driver patch
can modify them.

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
v2:
- fix capture in bad variable
- ignore doc/ and config/
---
 scripts/check-git-log.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/scripts/check-git-log.sh b/scripts/check-git-log.sh
index 833aa39..21f1055 100755
--- a/scripts/check-git-log.sh
+++ b/scripts/check-git-log.sh
@@ -70,6 +70,24 @@ bad=$(echo "$headlines" | grep --color=always \
 	| sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong headline format:\n$bad\n"
 
+# check headline prefix when touching only drivers, e.g. net/<driver name>
+bad=$(for commit in $commits ; do
+	headline=$(git log --format='%s' -1 $commit)
+	files=$(git diff-tree --no-commit-id --name-only -r $commit)
+	[ -z "$(echo "$files" | grep -v '^\(drivers\|doc\|config\)/')" ] ||
+		continue
+	drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f 2,3 | sort -u)
+	drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq)
+	if [ $(echo "$drvgrp" | wc -l) -gt 1 ] ; then
+		echo "$headline" | grep -v '^drivers:'
+	elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then
+		echo "$headline" | grep -v "^$drvgrp"
+	else
+		echo "$headline" | grep -v "^$drv"
+	fi
+done | sed 's,^,\t,')
+[ -z "$bad" ] || printf "Wrong headline prefix:\n$bad\n"
+
 # check headline label for common typos
 bad=$(echo "$headlines" | grep --color=always \
 	-e '^example[:/]' \
-- 
2.7.0

Re: [dpdk-dev] [PATCH 0/3] more commits checks

[Thomas Monjalon]

2016-07-06 12:38, Thomas Monjalon:
> The script check-git-log.sh helps to keep a consistent git history.
> Scrolling/parsing the history is easier when we are used to some
> rules.
> 
> Nélio Laranjeiro (1):
>   scripts: check headline of drivers commits
> 
> Thomas Monjalon (2):
>   scripts: check spacing after commit references
>   scripts: reduce line size of commit checks

Series applied (with 3/3 v2)