> -----Original Message-----
> From: jpali...@marvell.com <jpali...@marvell.com>
> Sent: Tuesday, July 19, 2022 2:01 PM
> To: Jakub Palider <jpali...@marvell.com>
> Cc: david.march...@redhat.com; dev@dpdk.org; tho...@monjalon.net
> Subject: [PATCH v3] devtools: ensure proper tag sequence
>
> From: Jakub Palider <jpali...@marvell.com>
>
> This change to log checking procedure ensures that certain tags are in proper
> order.
> The order of tags is as follows:
> * Coverity issue
> * Bugzilla ID
> * Fixes
> * Cc
> * <BLANK LINE>
> * Suggested-by
> * Reported-by
> + Signed-off-by
> * Acked-by
> * Reviewed-by
> * Tested-by
> where:
> * => 0 or more than one instance possible + => more than once instance
> possible In order to satisfy the above requirements an extra check is
> performed for obligatory tags.
>
> v3:
> * restored some tags under check
> * defined chronological order after Signed-off
>
> v2:
> * narrowed down the tags under check
>
> Signed-off-by: Jakub Palider <jpali...@marvell.com>
> ---
> devtools/check-git-log.sh | 56 +++++++++++++++++++++++++++++
> doc/guides/contributing/patches.rst | 28 +++++++++++++++
> 2 files changed, 84 insertions(+)
>
> diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh index
> 23c6a7d9bb..9b98207f3a 100755
> --- a/devtools/check-git-log.sh
> +++ b/devtools/check-git-log.sh
> @@ -54,6 +54,7 @@ fixes=$(git log --format='%h %s' --reverse $range | grep
> -i ': *fix' | cut -d' '
> stablefixes=$($selfdir/git-log-fixes.sh $range | sed '/(N\/A)$/d' | cut -d'
> ' -
> f2) tags=$(git log --format='%b' --reverse $range | grep -i -e 'by *:' -e
> 'fix.*:')
> bytag='\(Reported\|Suggested\|Signed-off\|Acked\|Reviewed\|Tested\)-
> by:'
> +exttag='Coverity issue:\|Bugzilla ID:\|Fixes:\|Cc:'
>
> failure=false
>
> @@ -203,6 +204,61 @@ done)
> [ -z "$bad" ] || { printf "Is it candidate for Cc: sta...@dpdk.org
> backport?\n$bad\n"\
> && failure=true;}
>
> +# check tag sequence
> +bad=$(for commit in $commits; do
> + body=$(git log --format='%b' -1 $commit)
> + echo "$body" |\
> + grep -o -e "$exttag\|^[[:blank:]]*$\|$bytag" | \
> + # retrieve tags only
> + cut -f1 -d":" |\
> + # it is okay to have several tags of the same type but for processing
> + # we need to squash them
> + uniq |\
> + # make sure the tags are in the proper order as presented in SEQ
> + awk -v cmt="$commit" 'BEGIN{
> + SEQ[0] = "Coverity issue";
> + SEQ[1] = "Bugzilla ID";
> + SEQ[2] = "Fixes";
> + SEQ[3] = "Cc";
> + SEQ[4] = "^$";
> + SEQ[5] = "Suggested-by";
> + SEQ[6] = "Reported-by";
> + SEQ[7] = "Signed-off-by";
> + SEQ[8] = "Acked-by";
> + SEQ[9] = "Reviewed-by";
> + SEQ[10] = "Tested-by";
> + latest = 0;
> + chronological = 0;
> + }
> + {
> + for (seq = 0; seq < length(SEQ); seq++) {
> + if (chronological == 1)
> + continue;
> + if (match($0, SEQ[seq])) {
> + if (seq < latest) {
> + print "\tCommit " cmt " (" $0 ":)";
> + break;
> + } else {
> + latest = seq;
> + }
> + }
> + }
> + if (match($0, "Signed-off-by"))
> + chronological = 1;
> + }'
> +done)
> +[ -z "$bad" ] || { printf "Wrong tag order: \n$bad\n"\
> + && failure=true;}
> +
> +# check required tag
> +bad=$(for commit in $commits; do
> + body=$(git log --format='%b' -1 $commit)
> + echo $body | grep -q "Signed-off-by:" \
> + || echo "\tCommit" $commit "(Signed-off-by:)"
> + done)
> +[ -z "$bad" ] || { printf "Missing obligatory tag: \n$bad\n"\
> + && failure=true;}
> +
> total=$(echo "$commits" | wc -l)
> if $failure ; then
> printf "\nInvalid patch(es) found - checked $total patch"
> diff --git a/doc/guides/contributing/patches.rst
> b/doc/guides/contributing/patches.rst
> index bebcaf3925..996089f2eb 100644
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -360,6 +360,34 @@ Where ``NNNNN`` is patchwork ID for patch or
> series::
> ---
> Depends-on: series-10000 ("Title of the series")
>
> +Tag order
> +~~~~~~~~~
> +
> +There is a pattern indicating how certain tags should relate to each other.
> +
> +Example of proper tag sequence::
> +
> + Coverity issue:
> + Bugzilla ID:
> + Fixes:
> + Cc:
> +
> + Suggested-by:
> + Reported-by:
> + Signed-off-by:
> + Acked-by:
> + Reviewed-by:
> + Tested-by:
> +
> +Between first and second tag section there is and empty line.
> +
> +While ``Signed-off-by:`` is an obligatory tag and must exists in each
> +commit, all other tags are optional. Any tag, as long as it is in
> +proper location to other adjacent tags (if present), may occur multiple
> times.
> +
> +Other tags after the first occurrence of ``Signed-off-by:`` shall be
> +laid out in a chronological order.
> +
> Creating Patches
> ----------------
>
> --
> 2.25.1
Hello,
as the automatic tests reported performance only degradation on patch analysis
change would it be feasible to integrate this patch?
I think all concerns from previous versions are addressed.
Regards,
Jakub
