All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dwaipayan Ray <dwaipayanray1@gmail.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>, Joe Perches <joe@perches.com>
Cc: linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] Addition of verbose mode to checkpatch
Date: Sat, 23 Jan 2021 19:41:02 +0530	[thread overview]
Message-ID: <CABJPP5B+bP6iVvSV9WoaF-kai5aOmi5K_toszFqAkjrmWp+emw@mail.gmail.com> (raw)
In-Reply-To: <CAKXUXMy532KTuzfE09dJ_Yi4JUV3ZR5cyj-zapkaysFtgN=saw@mail.gmail.com>

On Sat, Jan 23, 2021 at 6:16 PM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
>
> Sounds good to me.
>
> Dwaipayan Ray <dwaipayanray1@gmail.com> schrieb am Sa., 23. Jan. 2021, 11:12:
>>
>> On Sat, Jan 23, 2021 at 8:44 AM Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
>> >
>> > On Fri, Jan 22, 2021 at 10:27 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
>> > >
>> > > Hi Lukas and all,
>> > > Recently while going through the warnings emitted by checkpatch,
>> > > the necessity of a verbose mode came up once again. Joe had
>> > > already suggested that a verbose mode would probably be worked
>> > > on.
>> > >
>> > > As for how that could be done, that leaves us at a couple of options.
>> > > Since writing verbose messages for all warnings aren't possible at once,
>> > > there can be an optional extension when emitting messages:
>> > >
>> > > Currently, a warning can be emitted by
>> > > WARN("TYPE", "Message")
>> > > which could be converted to say:
>> > > WARN('TYPE", "Message", "Verbose")
>> > >
>> > > Another way is to leave the original warning emitting syntax intact
>> > > and instead go for a dictionary for verbose messages:
>> > > our %dict = (
>> > > "TYPE1" => "Verbose",
>> > > "TYPE2" => "Verbose"
>> > > ...);
>> > >
>> > > Although this leaves us the ability to customize the verbose output
>> > > for each warning of a particular type.
>> > >
>> > > Which do you think would be best? Certainly more options might be
>> > > possible, so any new inputs will be nice as well!
>> > >
>> >
>> > I think any of the two approaches would work.
>> >
>> > But how about a completely different approach?
>> >
>> > The verbose message is really an explanation of why kernel developers
>> > think this rule should be followed (what is the history, what does it
>> > try to warn about, what are possible fixes, what are accepted
>> > deviations, ...).
>> >
>> > So, this should not be placed into the checkpatch.pl script itself,
>> > but into Documentation.
>> >
>> > checkpatch.pl would simply quickly parse the Documentation and present
>> > the content for the specific rules that were violated.
>> >
>> > Let's say this Documentation is in
>> > ./Documentation/tools/checkpatch.rst, and they contain subsections
>> > with labels/name with the TYPE identifier. Then when a warning of TYPE
>> > is relevant for a specific patch, you print the description from that
>> > one subsection.
>> >
>> > In checkpatch itself, you would only need a very generic mechanism then.
>> >
>>
>> That sounds great to me!
>> Even I had it in mind but thought maybe checkpatch could be less
>> reliant on other
>> files. But this also solves our purpose for an external documentation
>> for checkpatch
>> rules, so its a kinda win win situation.
>>
>> I think maybe we will need a special structure for the documentation file itself
>> but its certainly doable.
>>
>> Do you think I should go on to propose this idea to Joe now?
>>

Hey Joe!
We came across some ideas for a verbose mode in checkpatch. Two basic
ideas were to modify the warning emitting syntax to include an optional
verbose text, say WARN("TYPE", "MESSAGE", "VERBOSE_MSG"), and
another was adding a dictionary of some sorts which contained the verbose
description of the types.

But the most interesting idea here is to use an external documentation file,
say Documentation/tools/checkpatch.rst which can be parsed by checkpatch
and consecutively displays the verbose messages while emitting warnings.
This serves two purposes:
o Adding the verbose mode
o Adding an external documentation for checkpatch

Is this idea acceptable to you?

Thank you,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

  reply	other threads:[~2021-01-23 14:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-22 21:27 [Linux-kernel-mentees] Addition of verbose mode to checkpatch Dwaipayan Ray
2021-01-23  3:14 ` Lukas Bulwahn
2021-01-23 10:11   ` Dwaipayan Ray
2021-01-23 12:45     ` Lukas Bulwahn
2021-01-23 14:11       ` Dwaipayan Ray [this message]
2021-01-23 16:59         ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CABJPP5B+bP6iVvSV9WoaF-kai5aOmi5K_toszFqAkjrmWp+emw@mail.gmail.com \
    --to=dwaipayanray1@gmail.com \
    --cc=joe@perches.com \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=lukas.bulwahn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.