> -----Original Message-----
> From: Thomas Monjalon <tho...@monjalon.net>
> Sent: Sunday, August 8, 2021 7:25 PM
> To: Xueming(Steven) Li <xuemi...@nvidia.com>
> Cc: dev@dpdk.org; sta...@dpdk.org; Christian Ehrhardt
> <christian.ehrha...@canonical.com>; bl...@debian.org;
> ktray...@redhat.com; david.march...@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 <tho...@monjalon.net>
> > Cc: sta...@dpdk.org
> > Cc: Christian Ehrhardt <christian.ehrha...@canonical.com>
> >
> > Signed-off-by: Xueming Li <xuemi...@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.
>
