DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log
@ 2019-10-25  9:59 Sean Morrissey
  2019-10-25 13:44 ` Ferruh Yigit
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sean Morrissey @ 2019-10-25  9:59 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Sean Morrissey

Moved title syntax to a separate file so that it improves code readability
and allows for easy addition of new correct title syntax in future cases.

Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
---
 devtools/check-git-log.sh        | 60 ++++++++------------------------
 devtools/commit-title-syntax.txt | 45 ++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 45 deletions(-)
 create mode 100644 devtools/commit-title-syntax.txt

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index a763ccf78..4152f6dfa 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -83,51 +83,21 @@ bad=$(echo "$headlines" | grep --color=always \
 	| sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
 
-# check headline uppercase (Rx/Tx, VF, L2, MAC, Linux, ARM...)
-bad=$(echo "$headlines" | grep -E --color=always \
-	-e ':.*\<(rx|tx|RX|TX)\>' \
-	-e ':.*\<[pv]f\>' \
-	-e ':.*\<[hsf]w\>' \
-	-e ':.*\<l[234]\>' \
-	-e ':.*\<api\>' \
-	-e ':.*\<ARM\>' \
-	-e ':.*\<(Aarch64|AArch64|AARCH64|Aarch32|AArch32|AARCH32)\>' \
-	-e ':.*\<(Armv7|ARMv7|ArmV7|armV7|ARMV7)\>' \
-	-e ':.*\<(Armv8|ARMv8|ArmV8|armV8|ARMV8)\>' \
-	-e ':.*\<crc\>' \
-	-e ':.*\<dcb\>' \
-	-e ':.*\<dma\>' \
-	-e ':.*\<eeprom\>' \
-	-e ':.*\<freebsd\>' \
-	-e ':.*\<iova\>' \
-	-e ':.*\<lacp\>' \
-	-e ':.*\<linux\>' \
-	-e ':.*\<lro\>' \
-	-e ':.*\<lsc\>' \
-	-e ':.*\<mac\>' \
-	-e ':.*\<mss\>' \
-	-e ':.*\<mtu\>' \
-	-e ':.*\<nic\>' \
-	-e ':.*\<nvm\>' \
-	-e ':.*\<numa\>' \
-	-e ':.*\<pci\>' \
-	-e ':.*\<phy\>' \
-	-e ':.*\<pmd\>' \
-	-e ':.*\<reta\>' \
-	-e ':.*\<rss\>' \
-	-e ':.*\<sctp\>' \
-	-e ':.*\<tos\>' \
-	-e ':.*\<tpid\>' \
-	-e ':.*\<tso\>' \
-	-e ':.*\<ttl\>' \
-	-e ':.*\<udp\>' \
-	-e ':.*\<[Vv]lan\>' \
-	-e ':.*\<vdpa\>' \
-	-e ':.*\<vsi\>' \
-	| grep \
-	-v ':.*\<OCTEON\ TX\>' \
-	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline lowercase:\n$bad\n"
+# check headline case (Rx/Tx, VF, L2, MAC, Linux ...)
+data="$selfdir/commit-title-syntax.txt"
+while IFS= read -r line
+do
+	regex=":.*\<$line\>"
+	bad=$(echo "$headlines" | grep -i $regex | grep \
+		-v ':.*\<OCTEON\ TX\>' )
+	if ! [ -z "$bad" ]
+	then
+		bad=$(echo "$headlines" | grep --color=always -v $regex \
+			| grep --color=always -i $regex \
+			| sed 's,^,\t,')
+		[ -z "$bad" ] || printf "Wrong headline case:\n$bad\n"
+	fi
+done < "$data"
 
 # special case check for VMDq to give good error message
 bad=$(echo "$headlines" | grep -E --color=always \
diff --git a/devtools/commit-title-syntax.txt b/devtools/commit-title-syntax.txt
new file mode 100644
index 000000000..0d4b9af01
--- /dev/null
+++ b/devtools/commit-title-syntax.txt
@@ -0,0 +1,45 @@
+Rx
+Tx
+PF
+VF
+HW
+SW
+FW
+L2
+L3
+L4
+API
+arm
+aarch64
+armv7
+armv8
+CRC
+DCB
+DMA
+EEPROM
+FreeBSD
+IOVA
+LACP
+Linux
+LRO
+LSC
+MAC
+MSS
+MTU
+NIC
+NVM
+NUMA
+PCI
+PHY
+PMD
+RETA
+RSS
+SCTP
+TOS
+TPID
+TSO
+TTL
+UDP
+VLAN
+VDPA
+VSI
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log
  2019-10-25  9:59 [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log Sean Morrissey
@ 2019-10-25 13:44 ` Ferruh Yigit
  2020-02-22 20:38 ` Thomas Monjalon
  2020-02-24 12:02 ` [dpdk-dev] [PATCH v2] devtools: export title syntax data " Ferruh Yigit
  2 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2019-10-25 13:44 UTC (permalink / raw)
  To: Sean Morrissey, Thomas Monjalon; +Cc: dev

On 10/25/2019 10:59 AM, Sean Morrissey wrote:
> Moved title syntax to a separate file so that it improves code readability
> and allows for easy addition of new correct title syntax in future cases.
> 
> Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log
  2019-10-25  9:59 [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log Sean Morrissey
  2019-10-25 13:44 ` Ferruh Yigit
@ 2020-02-22 20:38 ` Thomas Monjalon
  2020-02-24 12:02 ` [dpdk-dev] [PATCH v2] devtools: export title syntax data " Ferruh Yigit
  2 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2020-02-22 20:38 UTC (permalink / raw)
  To: Sean Morrissey; +Cc: dev, ferruh.yigit

Hi,

Sorry for not looking at this patch before.

25/10/2019 11:59, Sean Morrissey:
> Moved title syntax to a separate file so that it improves code readability
> and allows for easy addition of new correct title syntax in future cases.

I think it is a good idea for a reason which is not said here:
the logic is switched from checking bad patterns to checking good wording.

> Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
[...]
> +data="$selfdir/commit-title-syntax.txt"

The file is processed to check specifically the case of words,
so it should be renamed to something like words-case.txt
The file need to be added to MAINTAINERS file as well.

The variable name "data" is too vague. I suggest "words".

> +while IFS= read -r line

We should avoid "while" loop because it is a sub-shell,
so it forbids setting global variables as it is done
in another patch collecting stats:
	https://patches.dpdk.org/patch/65243/

"for" loop should be fine.

The variable name should be "word" I think.

> +do
> +	regex=":.*\<$line\>"
> +	bad=$(echo "$headlines" | grep -i $regex | grep \

If using -o option of grep, we can capture the word and compare directly
with the correct word. It will be also better when printing the error.

> +		-v ':.*\<OCTEON\ TX\>' )

I don't like having this exception but I have no better idea.
We can move it before the first grep so it will still work with grep -o.

> +	if ! [ -z "$bad" ]
> +	then
> +		bad=$(echo "$headlines" | grep --color=always -v $regex \
> +			| grep --color=always -i $regex \
> +			| sed 's,^,\t,')
> +		[ -z "$bad" ] || printf "Wrong headline case:\n$bad\n"

As said above, the word can be printed alone if using grep -o.

> +	fi
> +done < "$data"
>  
>  # special case check for VMDq to give good error message
>  bad=$(echo "$headlines" | grep -E --color=always \

The VMDq case should be moved as well.

> --- /dev/null
> +++ b/devtools/commit-title-syntax.txt
> @@ -0,0 +1,45 @@
> +Rx
> +Tx
> +PF
> +VF
> +HW
> +SW
> +FW
> +L2
> +L3
> +L4
> +API
> +arm

The right syntax is Arm.

> +aarch64

aarch32 is missing.

> +armv7
> +armv8
> +CRC
> +DCB
> +DMA
> +EEPROM
> +FreeBSD
> +IOVA
> +LACP
> +Linux
> +LRO
> +LSC
> +MAC
> +MSS
> +MTU
> +NIC
> +NVM
> +NUMA
> +PCI
> +PHY
> +PMD
> +RETA
> +RSS
> +SCTP
> +TOS
> +TPID
> +TSO
> +TTL
> +UDP
> +VLAN
> +VDPA

The right syntax is vDPA

> +VSI



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v2] devtools: export title syntax data for check-git-log
  2019-10-25  9:59 [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log Sean Morrissey
  2019-10-25 13:44 ` Ferruh Yigit
  2020-02-22 20:38 ` Thomas Monjalon
@ 2020-02-24 12:02 ` Ferruh Yigit
  2020-02-24 13:39   ` Thomas Monjalon
  2020-02-24 15:30   ` [dpdk-dev] [PATCH v3] " Ferruh Yigit
  2 siblings, 2 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-02-24 12:02 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Ferruh Yigit, Sean Morrissey

From: Sean Morrissey <sean.morrissey@intel.com>

Moved title syntax to a separate file so that it improves code
readability and allows easy addition.

Also logic changed from checking for bad pattern to checking good
pattern which documents the expected syntax more clearly, and does not
have gaps in the checks.

Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v2:
* renamed data file to words-case.txt and added to MAINTAINERS file
* Updated script
* as of now vdpa as file prefix is giving false positive
---
 MAINTAINERS               |  1 +
 devtools/check-git-log.sh | 63 ++++++++-------------------------------
 devtools/words-case.txt   | 48 +++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 51 deletions(-)
 create mode 100644 devtools/words-case.txt

diff --git a/MAINTAINERS b/MAINTAINERS
index 1886b18c3..923b49680 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -95,6 +95,7 @@ F: devtools/check-dup-includes.sh
 F: devtools/check-maintainers.sh
 F: devtools/check-forbidden-tokens.awk
 F: devtools/check-git-log.sh
+F: devtools/words-case.txt
 F: devtools/check-includes.sh
 F: devtools/check-symbol-maps.sh
 F: devtools/checkpatches.sh
diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index f9d055039..2ee5eefd7 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -83,57 +83,18 @@ bad=$(echo "$headlines" | grep --color=always \
 	| sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
 
-# check headline uppercase (Rx/Tx, VF, L2, MAC, Linux, ARM...)
-bad=$(echo "$headlines" | grep -E --color=always \
-	-e ':.*\<(rx|tx|RX|TX)\>' \
-	-e ':.*\<[pv]f\>' \
-	-e ':.*\<[hsf]w\>' \
-	-e ':.*\<l[234]\>' \
-	-e ':.*\<api\>' \
-	-e ':.*\<ARM\>' \
-	-e ':.*\<(Aarch64|AArch64|AARCH64|Aarch32|AArch32|AARCH32)\>' \
-	-e ':.*\<(Armv7|ARMv7|ArmV7|armV7|ARMV7)\>' \
-	-e ':.*\<(Armv8|ARMv8|ArmV8|armV8|ARMV8)\>' \
-	-e ':.*\<crc\>' \
-	-e ':.*\<dcb\>' \
-	-e ':.*\<dma\>' \
-	-e ':.*\<eeprom\>' \
-	-e ':.*\<freebsd\>' \
-	-e ':.*\<iova\>' \
-	-e ':.*\<lacp\>' \
-	-e ':.*\<linux\>' \
-	-e ':.*\<lro\>' \
-	-e ':.*\<lsc\>' \
-	-e ':.*\<mac\>' \
-	-e ':.*\<mss\>' \
-	-e ':.*\<mtu\>' \
-	-e ':.*\<nic\>' \
-	-e ':.*\<nvm\>' \
-	-e ':.*\<numa\>' \
-	-e ':.*\<pci\>' \
-	-e ':.*\<phy\>' \
-	-e ':.*\<pmd\>' \
-	-e ':.*\<reta\>' \
-	-e ':.*\<rss\>' \
-	-e ':.*\<sctp\>' \
-	-e ':.*\<tos\>' \
-	-e ':.*\<tpid\>' \
-	-e ':.*\<tso\>' \
-	-e ':.*\<ttl\>' \
-	-e ':.*\<udp\>' \
-	-e ':.*\<[Vv]lan\>' \
-	-e ':.*\<vdpa\>' \
-	-e ':.*\<vsi\>' \
-	| grep \
-	-v ':.*\<OCTEON\ TX\>' \
-	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline lowercase:\n$bad\n"
-
-# special case check for VMDq to give good error message
-bad=$(echo "$headlines" | grep -E --color=always \
-	-e '\<(vmdq|VMDQ)\>' \
-	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline capitalization, use 'VMDq':\n$bad\n"
+# check headline case (Rx/Tx, VF, L2, MAC, Linux ...)
+words="$selfdir/words-case.txt"
+for word in $(cat $words); do
+	bad=$(echo "$headlines" | grep -iw $word | grep -v $word)
+	if [ "$word" = "Tx" ]; then
+		bad=$(echo $bad | grep -v 'OCTEON\ TX')
+	fi
+	if [ -n "$bad" ]; then
+		bad_word=$(echo $bad | grep -io $word)
+		printf "Wrong headline case:\n\"$bad\": $bad_word --> $word\n"
+	fi
+done
 
 # check headline length (60 max)
 bad=$(echo "$headlines" |
diff --git a/devtools/words-case.txt b/devtools/words-case.txt
new file mode 100644
index 000000000..5d5490577
--- /dev/null
+++ b/devtools/words-case.txt
@@ -0,0 +1,48 @@
+OCTEON_TX
+aarch32
+aarch64
+API
+Arm
+armv7
+armv8
+CRC
+DCB
+DMA
+EEPROM
+FreeBSD
+FW
+HW
+IOVA
+L2
+L3
+L4
+LACP
+Linux
+LRO
+LSC
+MAC
+MSS
+MTU
+NIC
+NUMA
+NVM
+PCI
+PF
+PHY
+PMD
+RETA
+RSS
+Rx
+SCTP
+SW
+TOS
+TPID
+TSO
+TTL
+Tx
+UDP
+vDPA
+VF
+VLAN
+VMDq
+VSI
-- 
2.24.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v2] devtools: export title syntax data for check-git-log
  2020-02-24 12:02 ` [dpdk-dev] [PATCH v2] devtools: export title syntax data " Ferruh Yigit
@ 2020-02-24 13:39   ` Thomas Monjalon
  2020-02-24 14:38     ` Ferruh Yigit
  2020-02-24 15:30   ` [dpdk-dev] [PATCH v3] " Ferruh Yigit
  1 sibling, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2020-02-24 13:39 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, Sean Morrissey

24/02/2020 13:02, Ferruh Yigit:
> From: Sean Morrissey <sean.morrissey@intel.com>
> 
> Moved title syntax to a separate file so that it improves code
> readability and allows easy addition.
> 
> Also logic changed from checking for bad pattern to checking good
> pattern which documents the expected syntax more clearly, and does not
> have gaps in the checks.
> 
> Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> v2:
> * renamed data file to words-case.txt and added to MAINTAINERS file
> * Updated script
> * as of now vdpa as file prefix is giving false positive

False positive can be avoided if filtering out what is before the first colon.




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v2] devtools: export title syntax data for check-git-log
  2020-02-24 13:39   ` Thomas Monjalon
@ 2020-02-24 14:38     ` Ferruh Yigit
  0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2020-02-24 14:38 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Sean Morrissey

On 2/24/2020 1:39 PM, Thomas Monjalon wrote:
> 24/02/2020 13:02, Ferruh Yigit:
>> From: Sean Morrissey <sean.morrissey@intel.com>
>>
>> Moved title syntax to a separate file so that it improves code
>> readability and allows easy addition.
>>
>> Also logic changed from checking for bad pattern to checking good
>> pattern which documents the expected syntax more clearly, and does not
>> have gaps in the checks.
>>
>> Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> v2:
>> * renamed data file to words-case.txt and added to MAINTAINERS file
>> * Updated script
>> * as of now vdpa as file prefix is giving false positive
> 
> False positive can be avoided if filtering out what is before the first colon.
> 

It can, wasn't sure to add more work/check just to cover this case.

Also it is possible to introduce additional check for 'vdpa' only, right now it
is the only case that hits the file prefix.

Do you have a preference?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v3] devtools: export title syntax data for check-git-log
  2020-02-24 12:02 ` [dpdk-dev] [PATCH v2] devtools: export title syntax data " Ferruh Yigit
  2020-02-24 13:39   ` Thomas Monjalon
@ 2020-02-24 15:30   ` Ferruh Yigit
  2020-02-25 20:24     ` Thomas Monjalon
  1 sibling, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2020-02-24 15:30 UTC (permalink / raw)
  To: dev, Thomas Monjalon; +Cc: Ferruh Yigit, Sean Morrissey

From: Sean Morrissey <sean.morrissey@intel.com>

Moved title syntax to a separate file so that it improves code
readability and allows easy addition.

Also logic changed from checking for bad pattern to checking good
pattern which documents the expected syntax more clearly, and does not
have gaps in the checks.

Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v2:
* renamed data file to words-case.txt and added to MAINTAINERS file
* Updated script
* as of now vdpa as file prefix is giving false positive

v3:
* add check excluding file prefix to address vdpa false positive
* remove OCTEON_TX line from words-case.txt which was a mistake
---
 MAINTAINERS               |  1 +
 devtools/check-git-log.sh | 69 +++++++++------------------------------
 devtools/words-case.txt   | 47 ++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 53 deletions(-)
 create mode 100644 devtools/words-case.txt

diff --git a/MAINTAINERS b/MAINTAINERS
index 1886b18c3..923b49680 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -95,6 +95,7 @@ F: devtools/check-dup-includes.sh
 F: devtools/check-maintainers.sh
 F: devtools/check-forbidden-tokens.awk
 F: devtools/check-git-log.sh
+F: devtools/words-case.txt
 F: devtools/check-includes.sh
 F: devtools/check-symbol-maps.sh
 F: devtools/checkpatches.sh
diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index f9d055039..4e65be0e4 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -83,57 +83,22 @@ bad=$(echo "$headlines" | grep --color=always \
 	| sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
 
-# check headline uppercase (Rx/Tx, VF, L2, MAC, Linux, ARM...)
-bad=$(echo "$headlines" | grep -E --color=always \
-	-e ':.*\<(rx|tx|RX|TX)\>' \
-	-e ':.*\<[pv]f\>' \
-	-e ':.*\<[hsf]w\>' \
-	-e ':.*\<l[234]\>' \
-	-e ':.*\<api\>' \
-	-e ':.*\<ARM\>' \
-	-e ':.*\<(Aarch64|AArch64|AARCH64|Aarch32|AArch32|AARCH32)\>' \
-	-e ':.*\<(Armv7|ARMv7|ArmV7|armV7|ARMV7)\>' \
-	-e ':.*\<(Armv8|ARMv8|ArmV8|armV8|ARMV8)\>' \
-	-e ':.*\<crc\>' \
-	-e ':.*\<dcb\>' \
-	-e ':.*\<dma\>' \
-	-e ':.*\<eeprom\>' \
-	-e ':.*\<freebsd\>' \
-	-e ':.*\<iova\>' \
-	-e ':.*\<lacp\>' \
-	-e ':.*\<linux\>' \
-	-e ':.*\<lro\>' \
-	-e ':.*\<lsc\>' \
-	-e ':.*\<mac\>' \
-	-e ':.*\<mss\>' \
-	-e ':.*\<mtu\>' \
-	-e ':.*\<nic\>' \
-	-e ':.*\<nvm\>' \
-	-e ':.*\<numa\>' \
-	-e ':.*\<pci\>' \
-	-e ':.*\<phy\>' \
-	-e ':.*\<pmd\>' \
-	-e ':.*\<reta\>' \
-	-e ':.*\<rss\>' \
-	-e ':.*\<sctp\>' \
-	-e ':.*\<tos\>' \
-	-e ':.*\<tpid\>' \
-	-e ':.*\<tso\>' \
-	-e ':.*\<ttl\>' \
-	-e ':.*\<udp\>' \
-	-e ':.*\<[Vv]lan\>' \
-	-e ':.*\<vdpa\>' \
-	-e ':.*\<vsi\>' \
-	| grep \
-	-v ':.*\<OCTEON\ TX\>' \
-	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline lowercase:\n$bad\n"
-
-# special case check for VMDq to give good error message
-bad=$(echo "$headlines" | grep -E --color=always \
-	-e '\<(vmdq|VMDQ)\>' \
-	| sed 's,^,\t,')
-[ -z "$bad" ] || printf "Wrong headline capitalization, use 'VMDq':\n$bad\n"
+# check headline case (Rx/Tx, VF, L2, MAC, Linux ...)
+IFS='
+'
+words="$selfdir/words-case.txt"
+for word in $(cat $words); do
+	bad=$(echo "$headlines" | grep -iw $word | grep -v $word)
+	if [ "$word" = "Tx" ]; then
+		bad=$(echo $bad | grep -v 'OCTEON\ TX')
+	fi
+	for bad_line in $bad; do
+		bad_word=$(echo $bad_line | cut -d":" -f2 | grep -io $word)
+		if [ -n "$bad_word" ]; then
+			printf "Wrong headline case:\n\"$bad_line\": $bad_word --> $word\n"
+		fi
+	done
+done
 
 # check headline length (60 max)
 bad=$(echo "$headlines" |
@@ -187,8 +152,6 @@ done)
 [ -z "$bad" ] || printf "Missing 'Fixes' tag:\n$bad\n"
 
 # check Fixes: reference
-IFS='
-'
 fixtags=$(echo "$tags" | grep '^Fixes: ')
 bad=$(for fixtag in $fixtags ; do
 	hash=$(echo "$fixtag" | sed 's,^Fixes: \([0-9a-f]*\).*,\1,')
diff --git a/devtools/words-case.txt b/devtools/words-case.txt
new file mode 100644
index 000000000..6bfb81985
--- /dev/null
+++ b/devtools/words-case.txt
@@ -0,0 +1,47 @@
+aarch32
+aarch64
+API
+Arm
+armv7
+armv8
+CRC
+DCB
+DMA
+EEPROM
+FreeBSD
+FW
+HW
+IOVA
+L2
+L3
+L4
+LACP
+Linux
+LRO
+LSC
+MAC
+MSS
+MTU
+NIC
+NUMA
+NVM
+PCI
+PF
+PHY
+PMD
+RETA
+RSS
+Rx
+SCTP
+SW
+TOS
+TPID
+TSO
+TTL
+Tx
+UDP
+vDPA
+VF
+VLAN
+VMDq
+VSI
-- 
2.24.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v3] devtools: export title syntax data for check-git-log
  2020-02-24 15:30   ` [dpdk-dev] [PATCH v3] " Ferruh Yigit
@ 2020-02-25 20:24     ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2020-02-25 20:24 UTC (permalink / raw)
  To: Ferruh Yigit, Sean Morrissey; +Cc: dev

24/02/2020 16:30, Ferruh Yigit:
> From: Sean Morrissey <sean.morrissey@intel.com>
> 
> Moved title syntax to a separate file so that it improves code
> readability and allows easy addition.
> 
> Also logic changed from checking for bad pattern to checking good
> pattern which documents the expected syntax more clearly, and does not
> have gaps in the checks.
> 
> Signed-off-by: Sean Morrissey <sean.morrissey@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> +words="$selfdir/words-case.txt"
> +for word in $(cat $words); do

I think we should exclude comments in the words file.

> +	bad=$(echo "$headlines" | grep -iw $word | grep -v $word)
> +	if [ "$word" = "Tx" ]; then
> +		bad=$(echo $bad | grep -v 'OCTEON\ TX')
> +	fi

We could try to manage such false positive automatically by excluding
valid patterns while patterns list is iterated.
If OCTEON TX is before Tx in the list, no confusion is possible.

> +	for bad_line in $bad; do
> +		bad_word=$(echo $bad_line | cut -d":" -f2 | grep -io $word)

The cut will ignore anything after a second colon.
Should not happen but a possible minor issue.

> +		if [ -n "$bad_word" ]; then
> +			printf "Wrong headline case:\n\"$bad_line\": $bad_word --> $word\n"
> +		fi
> +	done
> +done

I think it is good enough to be merged.
We can adress above improvements in a separate patch.

Applied, thanks



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-02-25 20:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-25  9:59 [dpdk-dev] [PATCH] devtools: export title syntax for check-git-log Sean Morrissey
2019-10-25 13:44 ` Ferruh Yigit
2020-02-22 20:38 ` Thomas Monjalon
2020-02-24 12:02 ` [dpdk-dev] [PATCH v2] devtools: export title syntax data " Ferruh Yigit
2020-02-24 13:39   ` Thomas Monjalon
2020-02-24 14:38     ` Ferruh Yigit
2020-02-24 15:30   ` [dpdk-dev] [PATCH v3] " Ferruh Yigit
2020-02-25 20:24     ` Thomas Monjalon

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git