Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <4d6856b33cc870ac655d801d066f04af6ebadcd7.camel@perches.com>
Subject: Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch
From:   Joe Perches <joe@perches.com>
To:     Dwaipayan Ray <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc:     linux-kernel-mentees@lists.linuxfoundation.org,
        linux-kernel <linux-kernel@vger.kernel.org>
Date:   Wed, 17 Feb 2021 02:37:37 -0800
In-Reply-To: <CABJPP5CaAavDMWy8xmQ8Bxpf5549v8fqOz8Zi=g7g6ZGnCAVmg@mail.gmail.com>
References: <20210213131513.51386-1-dwaipayanray1@gmail.com>
         <20210213131513.51386-3-dwaipayanray1@gmail.com>
         <b2e25e683a3e2f31d4dc2d666e082634e30643b3.camel@perches.com>
         <CABJPP5CaAavDMWy8xmQ8Bxpf5549v8fqOz8Zi=g7g6ZGnCAVmg@mail.gmail.com>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.38.1-1 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 2021-02-16 at 19:48 +0530, Dwaipayan Ray wrote:
> On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
> > On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > > Add documentation for kernel script checkpatch.pl.
> > > This documentation is also parsed by checkpatch to
> > > enable a verbose mode.
> > > 
> > > The message types in checkpatch are documented with rst
> > > field lists. A total of 33 checkpatch type descriptions
> > > are added.
> > 
> > Alphabetic ordering isn't that great for these entries.
> > Please group them by use:
> > 
> > whitespace/code layout style:
> > SPACING, TRAILING_WHITESPACE, LINE_SPACING
[]
> Could I get some comment on this grouping for types:
> 
> Allocation Style: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
> 
> API Usage: ARCH_DEFINES, ARCH_INCLUDE_LINUX, ARRAY_SIZE, AVOID_BUG,
> ????????????????????AVOID_EXTERNS, AVOID_L_PREFIX, BIT_MACRO, CONSIDER_KSTRTO
> 
> Comment Style: BLOCK_COMMENT_STYLE, C99_COMMENTS
> 
> Commit Message: BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL,
> ??????????????????????????????COMMIT_MESSAGE, MISSING_SIGN_OFF,
> NO_AUTHOR_SIGN_OFF
> 
> Comparison Style: ASSIGN_IN_IF, BOOL_COMPARISON, COMPARISON_TO_NULL,
> ???????????????????????????????CONSTANT_COMPARISON
> 
> Spacing & Brackets: ASSIGNMENT_CONTINUATIONS, BRACES, BRACKET_SPACE,
> ??????????????????????????????????CODE_INDENT, CONCATENATED_STRING,
> LINE_SPACING,
> ??????????????????????????????????TRAILING_WHITESPACE
> 
> Others: CAMELCASE, CONFIG_DESCRIPTION
> 
> This is what I have done till now. Any suggestions would be nice and if it looks
> okay I would like to send the v4 in.

Looks OK.

Please make sure you at least include SPACING in the spacing & brackets
descriptions.

It also seems like ref links to Documentation/process/coding-style.rst
<section> (3.1 in the SPACING case) should be used more frequently.

It'd be 'nice' to somehow use sortable tables with some grouping
attribute for these groups, but I have no idea if that's feasible with
.rst restrutured text files.

Perhaps simplify the checkpatch code a bit for the --terse and --verbose
output.

Maybe something like:
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 869d80397f9f..07566cb3b3f8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl

@@ -292,15 +292,16 @@ GetOptions(
 
 help(0) if ($help);
 
+die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
+die "$P: --verbose canot be used with --terse\n" if ($verbose && $terse);
+
 list_types(0) if ($list_types);