Re: [PATCH v5 01/25] net: move intel drivers to intel subdirectory

[Thomas Monjalon <thomas@monjalon.net>]

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 :)