[v2,1/2] devtools: standardize script arguments
Checks
Commit Message
This patch modifies the arguments expected by the check-git-log script,
to match the format of arguments for the checkpatches script. Both
scripts now take certain argument options in the same format, making
them easier to use.
e.g. Both now take a commit ID range by "-r <range>"
The checkpatches help print is also updated to include the "-h" option.
Signed-off-by: Ciara Power <ciara.power@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
v2: Added exit 0 after print usage.
---
devtools/check-git-log.sh | 33 ++++++++++++++++++++++++---------
devtools/checkpatches.sh | 2 +-
2 files changed, 25 insertions(+), 10 deletions(-)
Comments
06/05/2020 11:55, Ciara Power:
> This patch modifies the arguments expected by the check-git-log script,
> to match the format of arguments for the checkpatches script. Both
> scripts now take certain argument options in the same format, making
> them easier to use.
> e.g. Both now take a commit ID range by "-r <range>"
[...]
> - usage: $(basename $0) [-h] [range]
> + usage: $(basename $0) [-h] [-nX|-r range]
Why not specifying that range can be also the first argument?
It is a discrepancy with what is documented in
doc/guides/contributing/patches.rst
> Check commit log formatting.
> - The git range can be specified as a "git log" option,
> - e.g. -1 to check only the latest commit.
> - The default range starts from origin/master to HEAD.
> + The git commits to be checked can be specified as a "git log" option,
> + by latest git commits limited with -n option, or commits in the git
> + range specified with -r option.
> + e.g. -n1 to check only the latest commit.
This line "e.g. -n1" looks disconnected from the above lines.
> + The default starts from origin/master to HEAD.
The rest looks OK.
Hi Thomas,
>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Sunday 24 May 2020 21:58
>To: Power, Ciara <ciara.power@intel.com>
>Cc: dev@dpdk.org
>Subject: Re: [dpdk-dev] [PATCH v2 1/2] devtools: standardize script
>arguments
>
>06/05/2020 11:55, Ciara Power:
>> This patch modifies the arguments expected by the check-git-log
>> script, to match the format of arguments for the checkpatches script.
>> Both scripts now take certain argument options in the same format,
>> making them easier to use.
>> e.g. Both now take a commit ID range by "-r <range>"
>[...]
>> - usage: $(basename $0) [-h] [range]
>> + usage: $(basename $0) [-h] [-nX|-r range]
>
>Why not specifying that range can be also the first argument?
>It is a discrepancy with what is documented in
>doc/guides/contributing/patches.rst
>
I didn't include the old format (that takes range as first argument) here to encourage
using the standardised format moving forward, but anyone that uses the old format
via scripts etc. will not see any difference in use.
I can also update the doc to show this standardised format to avoid discrepancy.
<snip>
Thanks,
Ciara
28/05/2020 16:37, Power, Ciara:
> From: Thomas Monjalon <thomas@monjalon.net>
> >06/05/2020 11:55, Ciara Power:
> >> This patch modifies the arguments expected by the check-git-log
> >> script, to match the format of arguments for the checkpatches script.
> >> Both scripts now take certain argument options in the same format,
> >> making them easier to use.
> >> e.g. Both now take a commit ID range by "-r <range>"
> >[...]
> >> - usage: $(basename $0) [-h] [range]
> >> + usage: $(basename $0) [-h] [-nX|-r range]
> >
> >Why not specifying that range can be also the first argument?
> >It is a discrepancy with what is documented in
> >doc/guides/contributing/patches.rst
> >
>
> I didn't include the old format (that takes range as first argument) here to encourage
> using the standardised format moving forward, but anyone that uses the old format
> via scripts etc. will not see any difference in use.
OK
So maybe just add a comment in the script on the old argument parsing
to mention it is the old syntax.
> I can also update the doc to show this standardised format to avoid discrepancy.
Yes please replace with the new syntax in the doc.
@@ -7,23 +7,38 @@
# If any doubt about the formatting, please check in the most recent history:
# git log --format='%>|(15)%cr %s' --reverse | grep -i <pattern>
-if [ "$1" = '-h' -o "$1" = '--help' ] ; then
+print_usage () {
cat <<- END_OF_HELP
- usage: $(basename $0) [-h] [range]
+ usage: $(basename $0) [-h] [-nX|-r range]
Check commit log formatting.
- The git range can be specified as a "git log" option,
- e.g. -1 to check only the latest commit.
- The default range starts from origin/master to HEAD.
+ The git commits to be checked can be specified as a "git log" option,
+ by latest git commits limited with -n option, or commits in the git
+ range specified with -r option.
+ e.g. -n1 to check only the latest commit.
+ The default starts from origin/master to HEAD.
END_OF_HELP
- exit
-fi
+}
selfdir=$(dirname $(readlink -f $0))
range=${1:-origin/master..}
+
+if [ "$range" = '--help' ] ; then
+ print_usage
+ exit 0
# convert -N to HEAD~N.. in order to comply with git-log-fixes.sh getopts
-if printf -- $range | grep -q '^-[0-9]\+' ; then
- range="HEAD$(printf -- $range | sed 's,^-,~,').."
+elif printf -- "$range" | grep -q '^-[0-9]\+' ; then
+ range="HEAD$(printf -- "$range" | sed 's,^-,~,').."
+else
+ while getopts hr:n: ARG ; do
+ case $ARG in
+ n ) range="HEAD~$OPTARG.." ;;
+ r ) range=$OPTARG ;;
+ h ) print_usage ; exit 0 ;;
+ ? ) print_usage ; exit 1 ;;
+ esac
+ done
+ shift $(($OPTIND - 1))
fi
commits=$(git log --format='%h' --reverse $range)
@@ -38,7 +38,7 @@ options="$options $DPDK_CHECKPATCH_OPTIONS"
print_usage () {
cat <<- END_OF_HELP
- usage: $(basename $0) [-q] [-v] [-nX|-r range|patch1 [patch2] ...]]
+ usage: $(basename $0) [-h] [-q] [-v] [-nX|-r range|patch1 [patch2] ...]
Run Linux kernel checkpatch.pl with DPDK options.
The environment variable DPDK_CHECKPATCH_PATH must be set.