[v2] checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs
diff mbox series

Message ID 20201218121133.18614-1-yashsri421@gmail.com
State New, archived
Headers show
Series
  • [v2] checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs
Related show

Commit Message

Aditya Srivastava Dec. 18, 2020, 12:11 p.m. UTC
Currently checkpatch warns for long line in commit messages even for
URL lines.

An evaluation over v4.13..v5.8 showed that out of 11729 warnings for
this class, around 299 are due to line starting with URL.

E.g., running checkpatch on commit 3cde818cd02b ("ASoC: topology:
Consolidate how dtexts and dvalues are freed") reports this warning:

WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html

Avoid giving users warning for character limit for such cases, instead
suggest them to prefix the URLs with "Link:"

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2:
- Fix coding style ('} else {')
- Make the URL check follow RFC 3986 style
- Give warning only if the URL is first non-whitespace of the line
- Set $commit_log_long_line only for else case
- Fix the warning count with exact figures and according to first non-space char as URL

 scripts/checkpatch.pl | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Aditya Srivastava Dec. 24, 2020, 3:19 p.m. UTC | #1
On 18/12/20 5:41 pm, Aditya Srivastava wrote:
> Currently checkpatch warns for long line in commit messages even for
> URL lines.
> 
> An evaluation over v4.13..v5.8 showed that out of 11729 warnings for
> this class, around 299 are due to line starting with URL.
> 
> E.g., running checkpatch on commit 3cde818cd02b ("ASoC: topology:
> Consolidate how dtexts and dvalues are freed") reports this warning:
> 
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
> 
> Avoid giving users warning for character limit for such cases, instead
> suggest them to prefix the URLs with "Link:"
> 
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2:
> - Fix coding style ('} else {')
> - Make the URL check follow RFC 3986 style
> - Give warning only if the URL is first non-whitespace of the line
> - Set $commit_log_long_line only for else case
> - Fix the warning count with exact figures and according to first non-space char as URL
> 
>  scripts/checkpatch.pl | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index abd5a3d2e913..bf77bd0b22cf 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3032,9 +3032,14 @@ sub process {
>  		      $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
>  					# A Fixes: or Link: line or signature tag line
>  		      $commit_log_possible_stack_dump)) {
> -			WARN("COMMIT_LOG_LONG_LINE",
> -			     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> -			$commit_log_long_line = 1;
> +			if ($line =~ /^\s*\b[a-z][\w\.\+\-]*:\/\/\S+/i) {
> +				WARN("COMMIT_LOG_LONG_LINE",
> +				     "Consider prefixing the URL with 'Link:'\n" . $herecurr);
> +			} else {
> +				WARN("COMMIT_LOG_LONG_LINE",
> +				     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> +				$commit_log_long_line = 1;
> +			}
>  		}
>  
>  # Reset possible stack dump if a blank line is found
> 

Hi Joe
You probably missed this patch. Please review :)

Thanks
Aditya

Patch
diff mbox series

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index abd5a3d2e913..bf77bd0b22cf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3032,9 +3032,14 @@  sub process {
 		      $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
 					# A Fixes: or Link: line or signature tag line
 		      $commit_log_possible_stack_dump)) {
-			WARN("COMMIT_LOG_LONG_LINE",
-			     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
-			$commit_log_long_line = 1;
+			if ($line =~ /^\s*\b[a-z][\w\.\+\-]*:\/\/\S+/i) {
+				WARN("COMMIT_LOG_LONG_LINE",
+				     "Consider prefixing the URL with 'Link:'\n" . $herecurr);
+			} else {
+				WARN("COMMIT_LOG_LONG_LINE",
+				     "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
+				$commit_log_long_line = 1;
+			}
 		}
 
 # Reset possible stack dump if a blank line is found