On Thu, Jan 23, 2025 at 5:02 PM Thomas Monjalon <tho...@monjalon.net> wrote: > > 23/01/2025 15:35, Bruce Richardson: > > On Thu, Jan 23, 2025 at 03:16:40PM +0100, David Marchand wrote: > > > On the check-git-log.sh update, we will have many warnings with current > > > update. > > > > > > Wrong headline prefix: > > > net/intel/common: add pkt reassembly fn for intel drivers > > > net/intel/common: provide common Tx entry structures > > > net/intel/common: add Tx mbuf ring replenish fn > > > net/intel: align Tx queue struct field names > > > net/intel: add prefix for driver-specific structs > > > net/intel/common: merge ice and i40e Tx queue struct > > > net/iavf: use common Tx queue structure > > > net/ixgbe: use common Tx queue structure > > > net/intel/common: pack Tx queue structure > > > net/intel/common: add post-Tx buffer free function > > > net/intel/common: add Tx buffer free fn for AVX-512 > > > net/iavf: use common Tx free fn for AVX-512 > > > net/ice: move Tx queue mbuf cleanup fn to common > > > net/iavf: use common Tx queue mbuf cleanup fn > > > net/ice: use vector SW ring for all vector paths > > > net/intel/common: remove unneeded code > > > net/intel/common: create common mbuf initializer fn > > > net/intel/common: extract common Rx vector criteria > > > > > > Invalid patch(es) found - checked 25 patches > > > > > > I tried to tweak this a bit, with the following heuristic: > > > * if touching only net/intel/common, accept net/intel/common: > > > * if touching multiple drivers under net/intel, then accept net/intel: > > > as prefix, > > > * if touching some net/intel/$drv (and optionnally net/intel/common), > > > accept net/$drv, > > > > > > diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh > > > index b2da013f6c..41c290f0ca 100755 > > > --- a/devtools/check-git-log.sh > > > +++ b/devtools/check-git-log.sh > > > @@ -79,11 +79,18 @@ bad=$(for commit in $commits ; do > > > [ -z "$(echo "$files" | grep -v '^\(drivers\|doc\|config\)/')" ] > > > || > > > continue > > > drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f 2,3 | sort > > > -u) > > > - # for drivers/net/intel/* use 2nd and 4th fields not 2nd and 3rd > > > if [ "$drv" = "net/intel" ] ; then > > > - drv=$(echo "$files" | grep '^drivers/' | cut -d "/" -f > > > 2,4 | sort -u) > > > + drvgrp="net/intel" > > > + drv=$(echo "$files" | grep '^drivers/' | grep -v > > > '^drivers/net/intel/common' | > > > + cut -d "/" -f 2,4 | sort -u) > > > + if [ $(echo "$drv" | wc -l) -eq 0 ] ; then > > > + drv='net/intel/common:' > > > + elif [ $(echo "$drv" | wc -l) -gt 1 ] ; then > > > + drv='net/intel:' > > > + fi > > > + else > > > + drvgrp=$(echo "$drv" | cut -d "/" -f 1 | uniq) > > > fi > > > - 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 > > > > > > Which then complains on patches in this series that touch many drivers > > > (but have net/intel/common: as prefix where I would suggest net/intel: > > > instead). > > > > > > > I tend to disagree with this suggestion. I think that a prefix can be valid > > so long as the prefix matches at least one component in the patch. For > > example, > > for the first patch in the set, I think net/intel/common is a better prefix > > than just "net/intel". I don't massively object to your suggestion, I just > > prefer patches identify the most relevant component, if possible, rather > > than generalities. > > Saying net/intel means the same thing as net/intel/common to me. > Why do we care whether the change is in common or multiple drivers? > At the end it impacts multiple Intel drivers. > The goal of the prefix is to quickly catch the scope of the change impact. > > One more argument: net/intel is shorter :)
True that any change on net/intel/common will affect many drivers. Whatever is decided, I just want the check to be less prone to false positives. With current series, output is a bit too vocal to me. -- David Marchand
