Skip to contentSkip to conversation

dim: cleanup checkpatch_commit

PATCH In review Jani Nikula 2 participants 9 messages Mar 13, 2018 – Mar 14, 2018

lore
Share & tools
lore

Jani Nikula

author
#0 lore raw

PATCH 1/3 dim: cleanup checkpatch_commit

Remove some old cruft. Pass checkpatch parameters via a variable. No functional changes.

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
dim +4 -3
index 45aec403a44e..81e2bc1511ac 100755
@@ -1360,13 +1360,14 @@ function check_maintainer
 # $1 is the git sha1 to check
 function checkpatch_commit
 {
-	local commit cmd bug_lines non_i915_files rv
+	local commit rv checkpatch_options
 
 	commit=$1
-	cmd="git show --pretty=email $commit"
+	checkpatch_options="-q --emacs --strict --show-types -"
 
 	git --no-pager log --oneline -1 $commit
-	if ! $cmd | scripts/checkpatch.pl -q --emacs --strict --show-types -; then
+	if ! git show --pretty=email $commit |\
+			scripts/checkpatch.pl $checkpatch_options; then
 		rv=1
 	fi
Signature · 6 lines
2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#1 lore raw

PATCH 2/3 dim: add checkpatch profiles to allow different checkpatch options

To reduce noise on CI checkpatch reports, we want to silence some checkpatch warnings. Different branches may end up having different rules, and users may want to get unfiltered results, so introduce checkpatch profiles. Add some placeholder profiles to be filled later on.

The idea is that CI would run 'dim checkpatch HEAD^ drm-intel' (or some other commit range-ish as the case may be).

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim     | 46 +++++++++++++++++++++++++++++++++++++++++-----
 dim.rst |  9 +++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)
dim +41 -5
index 81e2bc1511ac..4ba1c7ff490a 100755
@@ -836,7 +836,7 @@ function apply_patch #patch_file
 		rv=1
 	fi
 
-	if ! checkpatch_commit HEAD; then
+	if ! checkpatch_commit HEAD branch; then
 		rv=1
 	fi
 	if ! check_maintainer $branch HEAD; then

@@ -1358,12 +1358,47 @@ function check_maintainer
 }
 
 # $1 is the git sha1 to check
+# $2 is the checkpatch profile
 function checkpatch_commit
 {
-	local commit rv checkpatch_options
+	local commit rv checkpatch_options profile profile_options
 
 	commit=$1
-	checkpatch_options="-q --emacs --strict --show-types -"
+	profile=${2:-default}
+
+	# special branch profile maps branches to profiles
+	if [[ "$profile" = "branch" ]]; then
+		case "$(git_current_branch)" in
+			drm-intel-next-queued|drm-intel-next-fixes|drm-intel-fixes)
+				profile=drm-intel
+				;;
+			drm-misc-next|drm-misc-next-fixes|drm-misc-fixes)
+				profile=drm-misc
+				;;
+			*)
+				profile=default
+				;;
+		esac
+	fi
+
+	# map profiles to checkpatch options
+	case "$profile" in
+		default)
+			profile_options=""
+			;;
+		drm-misc)
+			profile_options=""
+			;;
+		drm-intel)
+			profile_options=""
+			;;
+		*)
+			echoerr "Unknown checkpatch profile $profile"
+			profile_options=""
+			;;
+	esac
+
+	checkpatch_options="-q --emacs --strict --show-types $profile_options -"
 
 	git --no-pager log --oneline -1 $commit
 	if ! git show --pretty=email $commit |\

@@ -1430,12 +1465,13 @@ function dim_extract_next_fixes
 dim_alias_cp=checkpatch
 function dim_checkpatch
 {
-	local range rv
+	local range profile rv
 
 	range=$(rangeish "${1:-}")
+	profile=${2:-}
 
 	for commit in $(git rev-list --reverse $range); do
-		if ! checkpatch_commit $commit; then
+		if ! checkpatch_commit $commit $profile; then
 			rv=1
 		fi
 	done
dim.rst +7 -2
index e2c5fd6e6d0a..cc930959e497 100644
@@ -130,13 +130,18 @@ fixes *commit-ish*
 Print the Fixes: and Cc: lines for the supplied *commit-ish* in the linux kernel
 CodingStyle approved format.
 
-checkpatch [*commit-ish* [.. *commit-ish*]]
--------------------------------------------
+checkpatch [*commit-ish* [.. *commit-ish*]] [*profile*]
+-------------------------------------------------------
 Runs the given commit range commit-ish..commit-ish through the check tools.
 
 If no commit-ish is passed, defaults to HEAD^..HEAD. If one commit-ish is passed
 instead of a range, the range commit-ish..HEAD is used.
 
+If profile is given, uses specific options for checkpatch error
+filtering. Current profiles are "default", "branch", "drm-intel", and
+"drm-misc". The "branch" profile maps the current git branch to the appropriate
+profile, or if the branch is not known, to "default".
+
 sparse [*commit-ish* [.. *commit-ish*]]
 ---------------------------------------
 Run sparse on the files changed by the given commit range.
Signature · 6 lines
2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 2/3
#2 lore raw

Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options

Quoted 44 lines · Jani Nikula

To reduce noise on CI checkpatch reports, we want to silence some checkpatch warnings. Different branches may end up having different rules, and users may want to get unfiltered results, so introduce checkpatch profiles. Add some placeholder profiles to be filled later on.

The idea is that CI would run 'dim checkpatch HEAD^ drm-intel' (or some other commit range-ish as the case may be).

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim     | 46 +++++++++++++++++++++++++++++++++++++++++-----
 dim.rst |  9 +++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)
dim +4 -3
index 81e2bc1511ac..4ba1c7ff490a 100755
@@ -836,7 +836,7 @@ function apply_patch #patch_file
 		rv=1
 	fi
 
-	if ! checkpatch_commit HEAD; then
+	if ! checkpatch_commit HEAD branch; then
 		rv=1
 	fi
 	if ! check_maintainer $branch HEAD; then

@@ -1358,12 +1358,47 @@ function check_maintainer
 }
 
 # $1 is the git sha1 to check
+# $2 is the checkpatch profile
 function checkpatch_commit
 {
-	local commit rv checkpatch_options
+	local commit rv checkpatch_options profile profile_options
 
 	commit=$1
-	checkpatch_options="-q --emacs --strict --show-types -"
+	profile=${2:-default}

dim function check_maintainer · line 1377

Quoted 10 lines
dim +10 -0
+
+	# special branch profile maps branches to profiles
+	if [[ "$profile" = "branch" ]]; then
+		case "$(git_current_branch)" in
+			drm-intel-next-queued|drm-intel-next-fixes|drm-intel-fixes)
+				profile=drm-intel
+				;;
+			drm-misc-next|drm-misc-next-fixes|drm-misc-fixes)
+				profile=drm-misc
+				;;

Use branch_to_repo instead, if that doesn't come up with anything, then default?

With that little bit of polished applied, on patches 1&2:

Reviewed-by Daniel Vetter <[email protected]>
Quoted 75 lines
dim +27 -2
+			*)
+				profile=default
+				;;
+		esac
+	fi
+
+	# map profiles to checkpatch options
+	case "$profile" in
+		default)
+			profile_options=""
+			;;
+		drm-misc)
+			profile_options=""
+			;;
+		drm-intel)
+			profile_options=""
+			;;
+		*)
+			echoerr "Unknown checkpatch profile $profile"
+			profile_options=""
+			;;
+	esac
+
+	checkpatch_options="-q --emacs --strict --show-types $profile_options -"
 
 	git --no-pager log --oneline -1 $commit
 	if ! git show --pretty=email $commit |\

@@ -1430,12 +1465,13 @@ function dim_extract_next_fixes
 dim_alias_cp=checkpatch
 function dim_checkpatch
 {
-	local range rv
+	local range profile rv
 
 	range=$(rangeish "${1:-}")
+	profile=${2:-}
 
 	for commit in $(git rev-list --reverse $range); do
-		if ! checkpatch_commit $commit; then
+		if ! checkpatch_commit $commit $profile; then
 			rv=1
 		fi
 	done
dim.rst +7 -2
index e2c5fd6e6d0a..cc930959e497 100644
@@ -130,13 +130,18 @@ fixes *commit-ish*
 Print the Fixes: and Cc: lines for the supplied *commit-ish* in the linux kernel
 CodingStyle approved format.
 
-checkpatch [*commit-ish* [.. *commit-ish*]]
--------------------------------------------
+checkpatch [*commit-ish* [.. *commit-ish*]] [*profile*]
+-------------------------------------------------------
 Runs the given commit range commit-ish..commit-ish through the check tools.
 
 If no commit-ish is passed, defaults to HEAD^..HEAD. If one commit-ish is passed
 instead of a range, the range commit-ish..HEAD is used.
 
+If profile is given, uses specific options for checkpatch error
+filtering. Current profiles are "default", "branch", "drm-intel", and
+"drm-misc". The "branch" profile maps the current git branch to the appropriate
+profile, or if the branch is not known, to "default".
+
 sparse [*commit-ish* [.. *commit-ish*]]
 ---------------------------------------
 Run sparse on the files changed by the given commit range.
Signature · 6 lines
2.11.0

_______________________________________________
dim-tools mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dim-tools
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#3 lore raw

Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options

Quoted 42 lines · Daniel Vetter
Quoted 41 lines · Jani Nikula

To reduce noise on CI checkpatch reports, we want to silence some checkpatch warnings. Different branches may end up having different rules, and users may want to get unfiltered results, so introduce checkpatch profiles. Add some placeholder profiles to be filled later on.

The idea is that CI would run 'dim checkpatch HEAD^ drm-intel' (or some other commit range-ish as the case may be).

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim     | 46 +++++++++++++++++++++++++++++++++++++++++-----
 dim.rst |  9 +++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)
dim +3 -2
index 81e2bc1511ac..4ba1c7ff490a 100755
@@ -836,7 +836,7 @@ function apply_patch #patch_file
 		rv=1
 	fi
 
-	if ! checkpatch_commit HEAD; then
+	if ! checkpatch_commit HEAD branch; then
 		rv=1
 	fi
 	if ! check_maintainer $branch HEAD; then

@@ -1358,12 +1358,47 @@ function check_maintainer
 }
 
 # $1 is the git sha1 to check
+# $2 is the checkpatch profile
 function checkpatch_commit
 {
-	local commit rv checkpatch_options
+	local commit rv checkpatch_options profile profile_options
Quoted 16 lines

dim function check_maintainer · line 1377

Quoted 13 lines
dim +11 -1
 	commit=$1
-	checkpatch_options="-q --emacs --strict --show-types -"
+	profile=${2:-default}
+
+	# special branch profile maps branches to profiles
+	if [[ "$profile" = "branch" ]]; then
+		case "$(git_current_branch)" in
+			drm-intel-next-queued|drm-intel-next-fixes|drm-intel-fixes)
+				profile=drm-intel
+				;;
+			drm-misc-next|drm-misc-next-fixes|drm-misc-fixes)
+				profile=drm-misc
+				;;

Use branch_to_repo instead, if that doesn't come up with anything, then default?

This command is supposed to work without configuration (a developer command)...

BR,
Jani.

Quoted 79 lines

With that little bit of polished applied, on patches 1&2:

Reviewed-by Daniel Vetter <[email protected]>
Quoted 75 lines
dim +27 -2
+			*)
+				profile=default
+				;;
+		esac
+	fi
+
+	# map profiles to checkpatch options
+	case "$profile" in
+		default)
+			profile_options=""
+			;;
+		drm-misc)
+			profile_options=""
+			;;
+		drm-intel)
+			profile_options=""
+			;;
+		*)
+			echoerr "Unknown checkpatch profile $profile"
+			profile_options=""
+			;;
+	esac
+
+	checkpatch_options="-q --emacs --strict --show-types $profile_options -"
 
 	git --no-pager log --oneline -1 $commit
 	if ! git show --pretty=email $commit |\

@@ -1430,12 +1465,13 @@ function dim_extract_next_fixes
 dim_alias_cp=checkpatch
 function dim_checkpatch
 {
-	local range rv
+	local range profile rv
 
 	range=$(rangeish "${1:-}")
+	profile=${2:-}
 
 	for commit in $(git rev-list --reverse $range); do
-		if ! checkpatch_commit $commit; then
+		if ! checkpatch_commit $commit $profile; then
 			rv=1
 		fi
 	done
dim.rst +7 -2
index e2c5fd6e6d0a..cc930959e497 100644
@@ -130,13 +130,18 @@ fixes *commit-ish*
 Print the Fixes: and Cc: lines for the supplied *commit-ish* in the linux kernel
 CodingStyle approved format.
 
-checkpatch [*commit-ish* [.. *commit-ish*]]
--------------------------------------------
+checkpatch [*commit-ish* [.. *commit-ish*]] [*profile*]
+-------------------------------------------------------
 Runs the given commit range commit-ish..commit-ish through the check tools.
 
 If no commit-ish is passed, defaults to HEAD^..HEAD. If one commit-ish is passed
 instead of a range, the range commit-ish..HEAD is used.
 
+If profile is given, uses specific options for checkpatch error
+filtering. Current profiles are "default", "branch", "drm-intel", and
+"drm-misc". The "branch" profile maps the current git branch to the appropriate
+profile, or if the branch is not known, to "default".
+
 sparse [*commit-ish* [.. *commit-ish*]]
 ---------------------------------------
 Run sparse on the files changed by the given commit range.
Signature · 6 lines
2.11.0

_______________________________________________
dim-tools mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dim-tools
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
#4 lore raw

Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options

Quoted 60 lines · Jani Nikula
Quoted 42 lines · Daniel Vetter
Quoted 41 lines · Jani Nikula

To reduce noise on CI checkpatch reports, we want to silence some checkpatch warnings. Different branches may end up having different rules, and users may want to get unfiltered results, so introduce checkpatch profiles. Add some placeholder profiles to be filled later on.

The idea is that CI would run 'dim checkpatch HEAD^ drm-intel' (or some other commit range-ish as the case may be).

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim     | 46 +++++++++++++++++++++++++++++++++++++++++-----
 dim.rst |  9 +++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)
dim +3 -2
index 81e2bc1511ac..4ba1c7ff490a 100755
@@ -836,7 +836,7 @@ function apply_patch #patch_file
             rv=1
     fi
 
-    if ! checkpatch_commit HEAD; then
+    if ! checkpatch_commit HEAD branch; then
             rv=1
     fi
     if ! check_maintainer $branch HEAD; then

@@ -1358,12 +1358,47 @@ function check_maintainer
 }
 
 # $1 is the git sha1 to check
+# $2 is the checkpatch profile
 function checkpatch_commit
 {
-    local commit rv checkpatch_options
+    local commit rv checkpatch_options profile profile_options
Quoted 16 lines

dim function check_maintainer · line 1377

Quoted 13 lines
dim +11 -1
     commit=$1
-    checkpatch_options="-q --emacs --strict --show-types -"
+    profile=${2:-default}
+
+    # special branch profile maps branches to profiles
+    if [[ "$profile" = "branch" ]]; then
+            case "$(git_current_branch)" in
+                    drm-intel-next-queued|drm-intel-next-fixes|drm-intel-fixes)
+                            profile=drm-intel
+                            ;;
+                    drm-misc-next|drm-misc-next-fixes|drm-misc-fixes)
+                            profile=drm-misc
+                            ;;

Use branch_to_repo instead, if that doesn't come up with anything, then default?

Quoted 2 lines

This command is supposed to work without configuration (a developer command)...

Duh :-/

I guess I'm ok as-is, but feels a bit silly.
-Daniel

Quoted 87 lines

BR,
Jani.

Quoted 75 lines

With that little bit of polished applied, on patches 1&2:

Reviewed-by Daniel Vetter <[email protected]>
Quoted 71 lines
dim +27 -2
+                    *)
+                            profile=default
+                            ;;
+            esac
+    fi
+
+    # map profiles to checkpatch options
+    case "$profile" in
+            default)
+                    profile_options=""
+                    ;;
+            drm-misc)
+                    profile_options=""
+                    ;;
+            drm-intel)
+                    profile_options=""
+                    ;;
+            *)
+                    echoerr "Unknown checkpatch profile $profile"
+                    profile_options=""
+                    ;;
+    esac
+
+    checkpatch_options="-q --emacs --strict --show-types $profile_options -"
 
     git --no-pager log --oneline -1 $commit
     if ! git show --pretty=email $commit |\

@@ -1430,12 +1465,13 @@ function dim_extract_next_fixes
 dim_alias_cp=checkpatch
 function dim_checkpatch
 {
-    local range rv
+    local range profile rv
 
     range=$(rangeish "${1:-}")
+    profile=${2:-}
 
     for commit in $(git rev-list --reverse $range); do
-            if ! checkpatch_commit $commit; then
+            if ! checkpatch_commit $commit $profile; then
                     rv=1
             fi
     done
dim.rst +7 -3
index e2c5fd6e6d0a..cc930959e497 100644
@@ -130,13 +130,18 @@ fixes *commit-ish*
 Print the Fixes: and Cc: lines for the supplied *commit-ish* in the linux kernel
 CodingStyle approved format.
 
-checkpatch [*commit-ish* [.. *commit-ish*]]
--------------------------------------------
+checkpatch [*commit-ish* [.. *commit-ish*]] [*profile*]
+-------------------------------------------------------
 Runs the given commit range commit-ish..commit-ish through the check tools.
 
 If no commit-ish is passed, defaults to HEAD^..HEAD. If one commit-ish is passed
 instead of a range, the range commit-ish..HEAD is used.
 
+If profile is given, uses specific options for checkpatch error
+filtering. Current profiles are "default", "branch", "drm-intel", and
+"drm-misc". The "branch" profile maps the current git branch to the appropriate
+profile, or if the branch is not known, to "default".
+
 sparse [*commit-ish* [.. *commit-ish*]]
 ---------------------------------------
 Run sparse on the files changed by the given commit range.
--

2.11.0

Quoted 4 lines
Quoted 4 lines

_______________________________________________
dim-tools mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dim-tools

--
Jani Nikula, Intel Open Source Technology Center

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#5 lore raw

Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options

Quoted 61 lines · Daniel Vetter
Quoted 60 lines · Jani Nikula
Quoted 58 lines · Daniel Vetter
Quoted 44 lines · Jani Nikula

To reduce noise on CI checkpatch reports, we want to silence some checkpatch warnings. Different branches may end up having different rules, and users may want to get unfiltered results, so introduce checkpatch profiles. Add some placeholder profiles to be filled later on.

The idea is that CI would run 'dim checkpatch HEAD^ drm-intel' (or some other commit range-ish as the case may be).

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim     | 46 +++++++++++++++++++++++++++++++++++++++++-----
 dim.rst |  9 +++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)
dim +4 -3
index 81e2bc1511ac..4ba1c7ff490a 100755
@@ -836,7 +836,7 @@ function apply_patch #patch_file
             rv=1
     fi
 
-    if ! checkpatch_commit HEAD; then
+    if ! checkpatch_commit HEAD branch; then
             rv=1
     fi
     if ! check_maintainer $branch HEAD; then

@@ -1358,12 +1358,47 @@ function check_maintainer
 }
 
 # $1 is the git sha1 to check
+# $2 is the checkpatch profile
 function checkpatch_commit
 {
-    local commit rv checkpatch_options
+    local commit rv checkpatch_options profile profile_options
 
     commit=$1
-    checkpatch_options="-q --emacs --strict --show-types -"
+    profile=${2:-default}

dim function check_maintainer · line 1377

Quoted 10 lines
dim +10 -0
+
+    # special branch profile maps branches to profiles
+    if [[ "$profile" = "branch" ]]; then
+            case "$(git_current_branch)" in
+                    drm-intel-next-queued|drm-intel-next-fixes|drm-intel-fixes)
+                            profile=drm-intel
+                            ;;
+                    drm-misc-next|drm-misc-next-fixes|drm-misc-fixes)
+                            profile=drm-misc
+                            ;;

Use branch_to_repo instead, if that doesn't come up with anything, then default?

Quoted 6 lines
Quoted 2 lines

This command is supposed to work without configuration (a developer command)...

Duh :-/

I guess I'm ok as-is, but feels a bit silly.

Thanks, pushed all three. I don't deny it feels a bit silly, but it's nothing that can't be fixed later.

BR,
Jani.

Quoted 89 lines

-Daniel

Quoted 87 lines

BR,
Jani.

Quoted 75 lines

With that little bit of polished applied, on patches 1&2:

Reviewed-by Daniel Vetter <[email protected]>
Quoted 71 lines
dim +27 -2
+                    *)
+                            profile=default
+                            ;;
+            esac
+    fi
+
+    # map profiles to checkpatch options
+    case "$profile" in
+            default)
+                    profile_options=""
+                    ;;
+            drm-misc)
+                    profile_options=""
+                    ;;
+            drm-intel)
+                    profile_options=""
+                    ;;
+            *)
+                    echoerr "Unknown checkpatch profile $profile"
+                    profile_options=""
+                    ;;
+    esac
+
+    checkpatch_options="-q --emacs --strict --show-types $profile_options -"
 
     git --no-pager log --oneline -1 $commit
     if ! git show --pretty=email $commit |\

@@ -1430,12 +1465,13 @@ function dim_extract_next_fixes
 dim_alias_cp=checkpatch
 function dim_checkpatch
 {
-    local range rv
+    local range profile rv
 
     range=$(rangeish "${1:-}")
+    profile=${2:-}
 
     for commit in $(git rev-list --reverse $range); do
-            if ! checkpatch_commit $commit; then
+            if ! checkpatch_commit $commit $profile; then
                     rv=1
             fi
     done
dim.rst +7 -3
index e2c5fd6e6d0a..cc930959e497 100644
@@ -130,13 +130,18 @@ fixes *commit-ish*
 Print the Fixes: and Cc: lines for the supplied *commit-ish* in the linux kernel
 CodingStyle approved format.
 
-checkpatch [*commit-ish* [.. *commit-ish*]]
--------------------------------------------
+checkpatch [*commit-ish* [.. *commit-ish*]] [*profile*]
+-------------------------------------------------------
 Runs the given commit range commit-ish..commit-ish through the check tools.
 
 If no commit-ish is passed, defaults to HEAD^..HEAD. If one commit-ish is passed
 instead of a range, the range commit-ish..HEAD is used.
 
+If profile is given, uses specific options for checkpatch error
+filtering. Current profiles are "default", "branch", "drm-intel", and
+"drm-misc". The "branch" profile maps the current git branch to the appropriate
+profile, or if the branch is not known, to "default".
+
 sparse [*commit-ish* [.. *commit-ish*]]
 ---------------------------------------
 Run sparse on the files changed by the given commit range.
--

2.11.0

Quoted 4 lines
Quoted 4 lines

_______________________________________________
dim-tools mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dim-tools

--
Jani Nikula, Intel Open Source Technology Center

Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#6 lore raw

PATCH 3/3 dim: loosen some drm-intel checkpatch rules

In reply to Jani Nikula

Set max line length to 100. I don't want to silence the LONG_LINE warning altogether, and I'd still prefer to keep lines under 80 characters, but I also don't want to see all the noise, and nor do I want to see silly code trying to arbitrarily squeeze under 80 when it doesn't make sense. 100 is a nice arbitrary round number... I hope review catches silly stuff regardless. Fingers crossed.

BIT_MACRO. We have (1 << N) all over the place. I hope to switch to BIT() macro eventually, but this documents current use.

PREFER_KERNEL_TYPES. We also have uint(8|16|32|64)_t all over the place. I also hope to move towards kernel types, but this documents current use.

SPLIT_STRING, LONG_LINE_STRING. Don't nag about strings split to many lines, but also don't nag about strings not split.

There's plenty more that could be tweaked, but let's start with something to improve the S/N ratio of the automated CI checkpatch reports. Now that we have --show-types included in the output, we can more easily discuss the ignores on a case-by-case basis.

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
dim +1 -1
index 4ba1c7ff490a..9fa6d9cd855b 100755
@@ -1390,7 +1390,7 @@ function checkpatch_commit
 			profile_options=""
 			;;
 		drm-intel)
-			profile_options=""
+			profile_options="--max-line-length=100 --ignore=BIT_MACRO,PREFER_KERNEL_TYPES,SPLIT_STRING,LONG_LINE_STRING"
 			;;
 		*)
 			echoerr "Unknown checkpatch profile $profile"
Signature · 6 lines
2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 3/3
#7 lore raw

Re: [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules

Quoted 35 lines · Jani Nikula

Set max line length to 100. I don't want to silence the LONG_LINE warning altogether, and I'd still prefer to keep lines under 80 characters, but I also don't want to see all the noise, and nor do I want to see silly code trying to arbitrarily squeeze under 80 when it doesn't make sense. 100 is a nice arbitrary round number... I hope review catches silly stuff regardless. Fingers crossed.

BIT_MACRO. We have (1 << N) all over the place. I hope to switch to BIT() macro eventually, but this documents current use.

PREFER_KERNEL_TYPES. We also have uint(8|16|32|64)_t all over the place. I also hope to move towards kernel types, but this documents current use.

SPLIT_STRING, LONG_LINE_STRING. Don't nag about strings split to many lines, but also don't nag about strings not split.

There's plenty more that could be tweaked, but let's start with something to improve the S/N ratio of the automated CI checkpatch reports. Now that we have --show-types included in the output, we can more easily discuss the ignores on a case-by-case basis.

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
dim +0 -0
index 4ba1c7ff490a..9fa6d9cd855b 100755

dim function checkpatch_commit · line 1393

Quoted 5 lines
dim +1 -1
@@ -1390,7 +1390,7 @@ function checkpatch_commit
 			profile_options=""
 			;;
 		drm-intel)
-			profile_options=""
+			profile_options="--max-line-length=100 --ignore=BIT_MACRO,PREFER_KERNEL_TYPES,SPLIT_STRING,LONG_LINE_STRING"

I've scrolled a bit through checkpatch complaines with this, and I think it looks a lot more reasonable. There's also a huge pile of stuff that we should probably have fixed when the patches landed, so CI'ing this looks good.

Reviewed-by Daniel Vetter <[email protected]>

Aside: Should we encourage checkpatch patches to clean this up, with the note that they must use the drm-intel profile and the caveat that we might want to add more stuff to our ignore list instead of taking the patches? -Daniel

Quoted 10 lines
dim +0 -0
 			;;
 		*)
 			echoerr "Unknown checkpatch profile $profile"
Signature · 6 lines
2.11.0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#8 lore raw

Re: [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules

Quoted 47 lines · Daniel Vetter
Quoted 35 lines · Jani Nikula

Set max line length to 100. I don't want to silence the LONG_LINE warning altogether, and I'd still prefer to keep lines under 80 characters, but I also don't want to see all the noise, and nor do I want to see silly code trying to arbitrarily squeeze under 80 when it doesn't make sense. 100 is a nice arbitrary round number... I hope review catches silly stuff regardless. Fingers crossed.

BIT_MACRO. We have (1 << N) all over the place. I hope to switch to BIT() macro eventually, but this documents current use.

PREFER_KERNEL_TYPES. We also have uint(8|16|32|64)_t all over the place. I also hope to move towards kernel types, but this documents current use.

SPLIT_STRING, LONG_LINE_STRING. Don't nag about strings split to many lines, but also don't nag about strings not split.

There's plenty more that could be tweaked, but let's start with something to improve the S/N ratio of the automated CI checkpatch reports. Now that we have --show-types included in the output, we can more easily discuss the ignores on a case-by-case basis.

Cc Arkadiusz Hiler <[email protected]>
Cc Joonas Lahtinen <[email protected]>
Cc Rodrigo Vivi <[email protected]>
Signed-off-by Jani Nikula <[email protected]>
---
 dim | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
dim +0 -0
index 4ba1c7ff490a..9fa6d9cd855b 100755

dim function checkpatch_commit · line 1393

Quoted 5 lines
dim +1 -1
@@ -1390,7 +1390,7 @@ function checkpatch_commit
 			profile_options=""
 			;;
 		drm-intel)
-			profile_options=""
+			profile_options="--max-line-length=100 --ignore=BIT_MACRO,PREFER_KERNEL_TYPES,SPLIT_STRING,LONG_LINE_STRING"

I've scrolled a bit through checkpatch complaines with this, and I think it looks a lot more reasonable. There's also a huge pile of stuff that we should probably have fixed when the patches landed, so CI'ing this looks good.

Quoted 5 lines
Reviewed-by Daniel Vetter <[email protected]>

Aside: Should we encourage checkpatch patches to clean this up, with the note that they must use the drm-intel profile and the caveat that we might want to add more stuff to our ignore list instead of taking the patches?

Overall I just think the checkpatch patches crop up enough without encouragement. IMO let's see how this rolls in CI first and proceed from there.

The two warnings that I do think would most benefit from mass change are BIT_MACRO and PREFER_KERNEL_TYPES. People tend to look around them and cargo cult. A little bit of git grep on the C99 types seems to back this up; their use multiplies and prospers in some files, while is neglible in others.

BR,
Jani.

Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx