Re: [PATCH v7 0/2] checkpatch: add verbose mode
On Thu, 2021-02-25 at 21:55 +0100, Mauro Carvalho Chehab wrote: > Em Thu, 25 Feb 2021 23:38:03 +0530 > Dwaipayan Ray escreveu: > > > On Thu, Feb 25, 2021 at 11:03 PM Joe Perches wrote: > > > > > > On Mon, 2021-02-22 at 13:22 +0530, Dwaipayan Ray wrote: > > > > Add a new verbose mode to checkpatch. The verbose test > > > > descriptions are read from the checkpatch documentation > > > > file at `Documentation/dev-tools/checkpatch.rst`. > > > > > > > > The verbose mode is optional and can be enabled by the > > > > flag -v or --verbose. > > > > > > > > The documentation file is only parsed by checkpatch.pl > > > > if the verbose mode is enabled. The verbose mode can > > > > not be used together with the --terse option. > > > > > > I don't have any real objection to this patch set, but as this > > > might be added to the Documentation tree and in .rst format, > > > perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have > > > some opinion. > > > > > > Also I do not want to be a maintainer of this .rst file and > > > likely neither Jon nor Mauro would either. Perhaps you? > > > > > > > > > > I could take it up if everybody is okay with it! > > > > > Ideally, the patch order would be reversed so the .rst file > > > is added first, then checkpatch updated to use it. > > > > > > > > > > Sure, if Jonathan or Mauro has no objections to it, I will be happy > > to resend it so that it can be picked up properly. > > I don't have any objections, provided that I won't be maintaining > it :-) > > - > > Just my two cents: > > IMO, maintaining this on a separate file can be a maintenance nightmare, > as this is the kind of thing that can become obsolete real soon. > > One alternative would be to use Pod::Usage module, just like > this script does: > > scripts/get_abi.pl > > with something similar to that, calling > > $ checkpatch --man > > Could generate a man-page style with all options, while: > > $ checkpatch --help > > would print the current help page. > > Yet, this would generate more work for Joe, as, for every new > type, the corresponding help text would be needed. Does this get integrated into the .rst output? I see: Documentation/Makefile:$(shell $(srctree)/scripts/get_abi.pl validate --dir $(srctree)/Documentation/ABI) But no obvious mechanism that emits .rst files for Pod::Usage And no, I'm not much interested in maintaining those docs either.
Re: [PATCH v7 0/2] checkpatch: add verbose mode
On Fri, Feb 26, 2021 at 11:29 AM Lukas Bulwahn wrote: > > On Thu, Feb 25, 2021 at 7:08 PM Dwaipayan Ray wrote: > > > > On Thu, Feb 25, 2021 at 11:03 PM Joe Perches wrote: > > > > > > On Mon, 2021-02-22 at 13:22 +0530, Dwaipayan Ray wrote: > > > > Add a new verbose mode to checkpatch. The verbose test > > > > descriptions are read from the checkpatch documentation > > > > file at `Documentation/dev-tools/checkpatch.rst`. > > > > > > > > The verbose mode is optional and can be enabled by the > > > > flag -v or --verbose. > > > > > > > > The documentation file is only parsed by checkpatch.pl > > > > if the verbose mode is enabled. The verbose mode can > > > > not be used together with the --terse option. > > > > > > I don't have any real objection to this patch set, but as this > > > might be added to the Documentation tree and in .rst format, > > > perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have > > > some opinion. > > > > > > Also I do not want to be a maintainer of this .rst file and > > > likely neither Jon nor Mauro would either. Perhaps you? > > > > > > > I could take it up if everybody is okay with it! > > > > And as I set Dwaipayan on this task on documenting checkpatch, I will > assist in maintaining this file as well. I will also pull some strings > to increase chances that Dwaipayan becomes a longer-term member in > this community and on this maintainer task. > Sounds nice to me! I would definitely love to remain as a active member even after the mentorship period ends. So I think this is a good start :) > > > Ideally, the patch order would be reversed so the .rst file > > > is added first, then checkpatch updated to use it. > > > > > > > Sure, if Jonathan or Mauro has no objections to it, I will be happy > > to resend it so that it can be picked up properly. > > > > > And _a lot_ more types and descriptive content should be added. > > > > Yes that's for sure. If this makes it I will try to get all of the > > other types in. > > I agree as well, probably a critical mass for inclusion is that we > have at least 25% (so roughly 50 rules) documented. > > > And if Lukas agrees, a little help from my fellow kernel mentees will > > be nice as well! > > > > Completely agree. I will recruit new mentees and go through the > exercises with them, until they are ready to send proper patches to > checkpatch.rst. As the designated maintainer of that file, you will be > busy reviewing, consolidating that content and pushing back if it is > not good enough for inclusion (so just as in the typical "good cop-bad > cop" game: I will motivate and help them to submit, you make sure you > get good content). > That is a nice plan! Certainly looking forward to it. Thanks & Regards, Dwaipayan.
Re: [PATCH v7 0/2] checkpatch: add verbose mode
On Fri, Feb 26, 2021 at 2:46 AM Jonathan Corbet wrote: > > Dwaipayan Ray writes: > > > On Thu, Feb 25, 2021 at 11:03 PM Joe Perches wrote: > >> I don't have any real objection to this patch set, but as this > >> might be added to the Documentation tree and in .rst format, > >> perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have > >> some opinion. > >> > >> Also I do not want to be a maintainer of this .rst file and > >> likely neither Jon nor Mauro would either. Perhaps you? > >> > > > > I could take it up if everybody is okay with it! > > > >> Ideally, the patch order would be reversed so the .rst file > >> is added first, then checkpatch updated to use it. > >> > > > > Sure, if Jonathan or Mauro has no objections to it, I will be happy > > to resend it so that it can be picked up properly. > > So I haven't been copied on this for a while. Looking in the archives, > I see that you have a manual table of contents at the top of the > document; you could take that out and let Sphinx generate it (and keep > it current!) for you. > Sure that sounds nice. I will modify the series and send the v8 in so that you can pick it up from there. That shall include the MAINTAINERS patch sent by Lukas as well. > Either way, though, if you want to merge this via some other path, it's > OK by me: > > Acked-by: Jonathan Corbet > Thanks for the ack! Regards, Dwaipayan. > Thanks, > > jon
Re: [PATCH v7 0/2] checkpatch: add verbose mode
On Thu, Feb 25, 2021 at 7:08 PM Dwaipayan Ray wrote: > > On Thu, Feb 25, 2021 at 11:03 PM Joe Perches wrote: > > > > On Mon, 2021-02-22 at 13:22 +0530, Dwaipayan Ray wrote: > > > Add a new verbose mode to checkpatch. The verbose test > > > descriptions are read from the checkpatch documentation > > > file at `Documentation/dev-tools/checkpatch.rst`. > > > > > > The verbose mode is optional and can be enabled by the > > > flag -v or --verbose. > > > > > > The documentation file is only parsed by checkpatch.pl > > > if the verbose mode is enabled. The verbose mode can > > > not be used together with the --terse option. > > > > I don't have any real objection to this patch set, but as this > > might be added to the Documentation tree and in .rst format, > > perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have > > some opinion. > > > > Also I do not want to be a maintainer of this .rst file and > > likely neither Jon nor Mauro would either. Perhaps you? > > > > I could take it up if everybody is okay with it! > And as I set Dwaipayan on this task on documenting checkpatch, I will assist in maintaining this file as well. I will also pull some strings to increase chances that Dwaipayan becomes a longer-term member in this community and on this maintainer task. > > Ideally, the patch order would be reversed so the .rst file > > is added first, then checkpatch updated to use it. > > > > Sure, if Jonathan or Mauro has no objections to it, I will be happy > to resend it so that it can be picked up properly. > > > And _a lot_ more types and descriptive content should be added. > > Yes that's for sure. If this makes it I will try to get all of the > other types in. I agree as well, probably a critical mass for inclusion is that we have at least 25% (so roughly 50 rules) documented. > And if Lukas agrees, a little help from my fellow kernel mentees will > be nice as well! > Completely agree. I will recruit new mentees and go through the exercises with them, until they are ready to send proper patches to checkpatch.rst. As the designated maintainer of that file, you will be busy reviewing, consolidating that content and pushing back if it is not good enough for inclusion (so just as in the typical "good cop-bad cop" game: I will motivate and help them to submit, you make sure you get good content). Lukas
Re: [PATCH v7 0/2] checkpatch: add verbose mode
Dwaipayan Ray writes: > On Thu, Feb 25, 2021 at 11:03 PM Joe Perches wrote: >> I don't have any real objection to this patch set, but as this >> might be added to the Documentation tree and in .rst format, >> perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have >> some opinion. >> >> Also I do not want to be a maintainer of this .rst file and >> likely neither Jon nor Mauro would either. Perhaps you? >> > > I could take it up if everybody is okay with it! > >> Ideally, the patch order would be reversed so the .rst file >> is added first, then checkpatch updated to use it. >> > > Sure, if Jonathan or Mauro has no objections to it, I will be happy > to resend it so that it can be picked up properly. So I haven't been copied on this for a while. Looking in the archives, I see that you have a manual table of contents at the top of the document; you could take that out and let Sphinx generate it (and keep it current!) for you. Either way, though, if you want to merge this via some other path, it's OK by me: Acked-by: Jonathan Corbet Thanks, jon
Re: [PATCH v7 0/2] checkpatch: add verbose mode
Em Thu, 25 Feb 2021 23:38:03 +0530 Dwaipayan Ray escreveu: > On Thu, Feb 25, 2021 at 11:03 PM Joe Perches wrote: > > > > On Mon, 2021-02-22 at 13:22 +0530, Dwaipayan Ray wrote: > > > Add a new verbose mode to checkpatch. The verbose test > > > descriptions are read from the checkpatch documentation > > > file at `Documentation/dev-tools/checkpatch.rst`. > > > > > > The verbose mode is optional and can be enabled by the > > > flag -v or --verbose. > > > > > > The documentation file is only parsed by checkpatch.pl > > > if the verbose mode is enabled. The verbose mode can > > > not be used together with the --terse option. > > > > I don't have any real objection to this patch set, but as this > > might be added to the Documentation tree and in .rst format, > > perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have > > some opinion. > > > > Also I do not want to be a maintainer of this .rst file and > > likely neither Jon nor Mauro would either. Perhaps you? > > > > I could take it up if everybody is okay with it! > > > Ideally, the patch order would be reversed so the .rst file > > is added first, then checkpatch updated to use it. > > > > Sure, if Jonathan or Mauro has no objections to it, I will be happy > to resend it so that it can be picked up properly. I don't have any objections, provided that I won't be maintaining it :-) - Just my two cents: IMO, maintaining this on a separate file can be a maintenance nightmare, as this is the kind of thing that can become obsolete real soon. One alternative would be to use Pod::Usage module, just like this script does: scripts/get_abi.pl with something similar to that, calling $ checkpatch --man Could generate a man-page style with all options, while: $ checkpatch --help would print the current help page. Yet, this would generate more work for Joe, as, for every new type, the corresponding help text would be needed. Thanks, Mauro
Re: [PATCH v7 0/2] checkpatch: add verbose mode
On Thu, Feb 25, 2021 at 11:03 PM Joe Perches wrote: > > On Mon, 2021-02-22 at 13:22 +0530, Dwaipayan Ray wrote: > > Add a new verbose mode to checkpatch. The verbose test > > descriptions are read from the checkpatch documentation > > file at `Documentation/dev-tools/checkpatch.rst`. > > > > The verbose mode is optional and can be enabled by the > > flag -v or --verbose. > > > > The documentation file is only parsed by checkpatch.pl > > if the verbose mode is enabled. The verbose mode can > > not be used together with the --terse option. > > I don't have any real objection to this patch set, but as this > might be added to the Documentation tree and in .rst format, > perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have > some opinion. > > Also I do not want to be a maintainer of this .rst file and > likely neither Jon nor Mauro would either. Perhaps you? > I could take it up if everybody is okay with it! > Ideally, the patch order would be reversed so the .rst file > is added first, then checkpatch updated to use it. > Sure, if Jonathan or Mauro has no objections to it, I will be happy to resend it so that it can be picked up properly. > And _a lot_ more types and descriptive content should be added. Yes that's for sure. If this makes it I will try to get all of the other types in. And if Lukas agrees, a little help from my fellow kernel mentees will be nice as well! With warm regards, Dwaipayan. > > > > Changes in v7: > > - Add color coding support to --list-types option > > > > Changes in v6: > > - Allow using verbose mode with --list-types option > > > > Changes in v5: > > - Change the reference format to use absolute links. > > - Print verbose descriptions only for the first time > > a message type is encountered. > > > > Changes in v4: > > - Change the type description format > > - Group the message types by usage > > - Make handling of --terse with --verbose simpler > > > > Changes in v3: > > - Simplify documentation file parsing in checkpatch > > - Document a total of 33 message types for checkpatch > > > > Changes in v2: > > - Use .rst Field Lists to specify the type descriptions. > > - Add a few more type descriptions to documentation. > > > > Dwaipayan Ray (2): > > checkpatch: add verbose mode > > docs: add documentation for checkpatch > > > > Documentation/dev-tools/checkpatch.rst | 525 + > > Documentation/dev-tools/index.rst | 1 + > > scripts/checkpatch.pl | 133 ++- > > 3 files changed, 639 insertions(+), 20 deletions(-) > > create mode 100644 Documentation/dev-tools/checkpatch.rst > > > >
Re: [PATCH v7 0/2] checkpatch: add verbose mode
On Mon, 2021-02-22 at 13:22 +0530, Dwaipayan Ray wrote: > Add a new verbose mode to checkpatch. The verbose test > descriptions are read from the checkpatch documentation > file at `Documentation/dev-tools/checkpatch.rst`. > > The verbose mode is optional and can be enabled by the > flag -v or --verbose. > > The documentation file is only parsed by checkpatch.pl > if the verbose mode is enabled. The verbose mode can > not be used together with the --terse option. I don't have any real objection to this patch set, but as this might be added to the Documentation tree and in .rst format, perhaps Jonathan Corbet and/or Mauro Carvalho Chehab might have some opinion. Also I do not want to be a maintainer of this .rst file and likely neither Jon nor Mauro would either. Perhaps you? Ideally, the patch order would be reversed so the .rst file is added first, then checkpatch updated to use it. And _a lot_ more types and descriptive content should be added. > > Changes in v7: > - Add color coding support to --list-types option > > Changes in v6: > - Allow using verbose mode with --list-types option > > Changes in v5: > - Change the reference format to use absolute links. > - Print verbose descriptions only for the first time > a message type is encountered. > > Changes in v4: > - Change the type description format > - Group the message types by usage > - Make handling of --terse with --verbose simpler > > Changes in v3: > - Simplify documentation file parsing in checkpatch > - Document a total of 33 message types for checkpatch > > Changes in v2: > - Use .rst Field Lists to specify the type descriptions. > - Add a few more type descriptions to documentation. > > Dwaipayan Ray (2): > checkpatch: add verbose mode > docs: add documentation for checkpatch > > Documentation/dev-tools/checkpatch.rst | 525 + > Documentation/dev-tools/index.rst | 1 + > scripts/checkpatch.pl | 133 ++- > 3 files changed, 639 insertions(+), 20 deletions(-) > create mode 100644 Documentation/dev-tools/checkpatch.rst >
