[Linux-kernel-mentees] [PATCH] checkpatch: ignore files not following NETWORKING_BLOCK_COMMENT_STYLE

Aditya yashsri421 at gmail.com
Mon Dec 21 15:29:14 UTC 2020


On 21/12/20 10:57 am, Lukas Bulwahn wrote:
> On Sun, Dec 20, 2020 at 7:33 PM Aditya Srivastava <yashsri421 at gmail.com> wrote:
>>
>> Currently checkpatch.pl gives warning for NETWORKING_BLOCK_COMMENT_STYLE
>> for files in net/ and drivers/net which do not follow the networking
>> comment style.
>> But some of these files seem to follow the generic comment style instead
>> of networking style and some rather mixed style of comment.
>>
>> For e.g., drivers/net/wireless/ralink/rt2x00 largely follows generic
>> kernel comment style in spite of being inside drivers/net.
>>
>> Provide an ignore file(".networking_block_comment_styles.ignore"), where
>> users can add the files they want to ignore this warning.
>>
> 
> I believe that is a really bad design decision here.
> 
> Imagine that in one directory, the maintainer wants to adjust ten
> rules for checkpatch.
> 
> Are we going to implement this logic for those ten rules individually?

No, only the files which are not following the networking comment
style, and are inside net/ or drivers/net should be added. The files
which are following the networking style already, need not add
themselves anywhere.

I feel, normally maintainer will want entire directory to follow the
same style. So, they can just add their directory in the .ignore file
and it will be ignored from the warning.

> Is the maintainer going to add
> 
> None of that is obviously acceptable: 1. because there are better
> design decisions; 2. because tailoring checkpatch would simply not be
> worth the cost of having all those individual files laying around in
> the repository and the complication throughout the whole script.
> 

We have earlier used a similar approach with ".get_maintainer.ignore",
where the users add their signature to be ignored from being mentioned
by "get_maintainers.pl" script.
Link: https://lore.kernel.org/patchwork/patch/939769/

We just need one file for our purpose (i.e., not necessarily .ignore
file).
Here I have used .ignore file in the root directory, so that it is
easier for the maintainer to edit it and add their directory in it.

If not in the root directory(as the files look scattered), perhaps we
can place it in script directory as .ignore or .txt file, or maybe
just place these directories in the form of a hash in the script
itself. But perhaps we'll need a list of directories to ignore and
then we'll be able to form the hash.

> Also, you already recognized that there is already a feature to ignore
> or select rules through command-line parameters.
> 
> So, how about coming up with a proper .checkpatch config file format
> and then use the already available feature to configure the checkpatch
> run?
> 

The user can add "--ignore NETWORKING_BLOCK_COMMENT_STYLE" line in
their .checkpatch.conf file, and checkpatch will then ignore this
class of warning.

Somewhat similar to this repository here:
https://github.com/openil/u-boot/blob/master/.checkpatch.conf

But this file(".checkpatch.conf") is ignored by git and is not
committed (as different users may require different configurations).
So every user will have to add this line in their individual config files.

Also, if the user starts working on a different file, where the
networking comment style is followed, he will again have to delete
this configuration line from the config file, to get this warning.

I'm not sure, if we can add directory/file specific configuration in
.checkpatch.conf. But again, if it was possible, we'll be specifying
individual files, and telling them to be avoided, similar to the
current approach.

Using any of these solution, if the users ignore this warning, we can
perhaps add a fix for NETWORKING_BLOCK_COMMENT_STYLE warning.

What do you think?

Thanks
Aditya

> Lukas
>


More information about the Linux-kernel-mentees mailing list