Re: [dpdk-dev] [dpdk-stable] [PATCH v2 2/2] devtools: fix patches missing if range newer than HEAD

["Xueming(Steven) Li" <xuemingl@nvidia.com>]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Sunday, August 8, 2021 7:25 PM
> To: Xueming(Steven) Li <xuemingl@nvidia.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Christian Ehrhardt <christian.ehrhardt@canonical.com>; bluca@debian.org;
> ktraynor@redhat.com; david.marchand@redhat.com
> Subject: Re: [dpdk-stable] [PATCH v2 2/2] devtools: fix patches missing if range newer than HEAD
> 
> 30/06/2021 08:34, Xueming Li:
> > Current fix scan scripts scanned specified range in HEAD branch.
> 
> I cannot parse the above sentence.

The scripts scans patches and try to get patch version info from current branch.

> 
> > When users ran it in an earlier branch, few patches were scanned due
> > to the fixes in the range are newer and not merged to HEAD branch.
> 
> You mean some patches were not scanned?

Scanned but failed to retrieve correct version info. 

> 
> >
> > This patch introduces optional <branch> argument, default to HEAD if
> > not specified. Checks the <range> specified in parameter must being
> > merged in <branch>.
> 
> Cannot parse either.
> 
> > Fixes: 752d8e097ec1 ("scripts: show fixes with release version of
> > bug")
> > Cc: Thomas Monjalon <thomas@monjalon.net>
> > Cc: stable@dpdk.org
> > Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> >
> > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> [...]
> > -	echo "usage: $(basename $0) [-h] <git_range>"
> > +	echo "usage: $(basename $0) [-h] <git_range> [<branch>]"
> [...]
> > -range="$*"
> > +range="$1"
> 
> I think it breaks passing range in multiple parameters without quotes.
> But it is not really a problem.
> 
> > +branch="$2"
> > +
> > +[ -n "$branch" ] || branch="HEAD"
> > +refbranch=$(git rev-parse --abbrev-ref $branch)
> 
> Why this line is needed? A comment may help.

OK

> If $branch is not used anymore, we can overwrite it instead of introducing one more variable $refbranch.

$branch will be used in function commit_version().

>
> > +range_last=$(git log --oneline $range |head -n1|cut -d' ' -f1)
> 
> spaces missing around pipes.
> You can avoid "head" and "cut" by providing the right options to git.

Yes, rev-parse will do this efficiently.

> 
> > +if ! git branch -a --contains $range_last |grep -q -e " $refbranch$" -e " remotes/$refbranch$"; then
> > +	echo "range $range not included by branch $refbranch"
> > +	exit 1
> > +fi
> >
> >  # get major release version of a commit  commit_version () # <hash>
> > {
> >  	local VER="v*.*"
> >  	# use current branch as history reference
> > -	local refbranch=$(git rev-parse --abbrev-ref HEAD)
> 
> You move a line but not its comment above.
>