All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Jonathan Corbet" <corbet@lwn.net>,
	"Tomasz Warniełło" <tomasz.warniello@gmail.com>
Cc: "Tomasz Warniełło" <tomasz.warniello@gmail.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scripts: kernel-doc: transform documentation into POD
Date: Fri, 17 Dec 2021 11:43:18 +0200	[thread overview]
Message-ID: <87k0g336tl.fsf@intel.com> (raw)
In-Reply-To: <87h7b8cfg0.fsf@meer.lwn.net>

On Thu, 16 Dec 2021, Jonathan Corbet <corbet@lwn.net> wrote:
> Tomasz Warniełło <tomasz.warniello@gmail.com> writes:
>
>> The only change in the script execution flow is the replacement
>> of the 'usage' function with the native core Perl 'pod2usage'.
>>
>> This entails:
>> - an overall documentation restructuring
>> - addition of a synopsis
>>
>> Otherwise my intervention is minimal:
>> - a few tiny language, formatting and spacing corrections
>> - a few missing bits added in the command syntax description
>> - adding subsections in the "FORMAT OF COMMENTS" section
>> - alphabetical sorting within OPTIONS subections
>
> So I think that this is generally a good thing, but I do have some
> quibbles.  Starting with the above, which is a pretty clear violation of
> the "each patch does one thing" rule.  Please separate out your changes
> into separate patches so that they are more easily reviewed.
>
> A few other things below...

I'd throw all code comment formatting documentation out of here, and
restrict the script to describing the command-line parameters only, and
focus on Documentation/doc-guide/kernel-doc.rst being the single point
of truth for comment formatting.


BR,
Jani.

>
>> Finally, the TODO stub evolves into a section:
>> - perldoc request removed
>> - undocumented options added
>>
>> Run `kernel-doc -h` to see the full doc.
>>
>> The TODO suggestion is ancient, thus I can't address its author with
>> a "Suggested-by" tag.
>>
>> Signed-off-by: Tomasz Warniełło <tomasz.warniello@gmail.com>
>> ---
>>  scripts/kernel-doc | 613 ++++++++++++++++++++++++++++++---------------
>>  1 file changed, 413 insertions(+), 200 deletions(-)
>>
>> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
>> index 3106b7536b89..00c0c7f5ff58 100755
>> --- a/scripts/kernel-doc
>> +++ b/scripts/kernel-doc
>> @@ -4,46 +4,33 @@
>>  use warnings;
>>  use strict;
>>  
>> -## Copyright (c) 1998 Michael Zucchi, All Rights Reserved        ##
>> -## Copyright (C) 2000, 1  Tim Waugh <twaugh@redhat.com>          ##
>> -## Copyright (C) 2001  Simon Huggins                             ##
>> -## Copyright (C) 2005-2012  Randy Dunlap                         ##
>> -## Copyright (C) 2012  Dan Luedtke                               ##
>> -## 								 ##
>> -## #define enhancements by Armin Kuster <akuster@mvista.com>	 ##
>> -## Copyright (c) 2000 MontaVista Software, Inc.			 ##
>
> My immediate reaction is that you shouldn't be removing copyright lines,
> though I did see that you put them back later.  I think, though, that
> the copyright assertions should remain at the top of the file; they
> don't need to be part of the help text that the program emits.  So leave
> them here, please.
>
> (I guess I should add one of my own, assuming I want any part of this
> file actually associated with my name...:)
>
>> -## This software falls under the GNU General Public License.     ##
>> -## Please read the COPYING file for more information             ##
>
> This could come out, though; that's what the SPDX line is for.
>
>> -# 18/01/2001 - 	Cleanups
>> -# 		Functions prototyped as foo(void) same as foo()
>> -# 		Stop eval'ing where we don't need to.
>> -# -- huggie@earth.li
>> -
>> -# 27/06/2001 -  Allowed whitespace after initial "/**" and
>> -#               allowed comments before function declarations.
>> -# -- Christian Kreibich <ck@whoop.org>
>> -
>> -# Still to do:
>> -# 	- add perldoc documentation
>> -# 	- Look more closely at some of the scarier bits :)
>> -
>> -# 26/05/2001 - 	Support for separate source and object trees.
>> -#		Return error code.
>> -# 		Keith Owens <kaos@ocs.com.au>
>> -
>> -# 23/09/2001 - Added support for typedefs, structs, enums and unions
>> -#              Support for Context section; can be terminated using empty line
>> -#              Small fixes (like spaces vs. \s in regex)
>> -# -- Tim Jansen <tim@tjansen.de>
>> -
>> -# 25/07/2012 - Added support for HTML5
>> -# -- Dan Luedtke <mail@danrl.de>
>
> These, too, should come out; that's what the git log is for.
>
> [...]
>
>>  my $kernelversion;
>> @@ -468,7 +306,7 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
>>      } elsif ($cmd eq "Werror") {
>>  	$Werror = 1;
>>      } elsif (($cmd eq "h") || ($cmd eq "help")) {
>> -	usage();
>> +			pod2usage(-exitval => 0, -verbose => 2);
>
> Why the strange indentation here?  This file is far from pretty, but
> that makes it worse.  (Other places too).
>
> [...]
>
> Thanks,
>
> jon

-- 
Jani Nikula, Intel Open Source Graphics Center

  reply	other threads:[~2021-12-17  9:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 22:55 [PATCH] scripts: kernel-doc: transform documentation into POD Tomasz Warniełło
2021-12-16 23:12 ` Jonathan Corbet
2021-12-17  9:43   ` Jani Nikula [this message]
2022-01-03  4:18   ` Tomasz Warniełło
2022-01-03  9:04     ` Jani Nikula
2022-01-03 17:30       ` Tomasz Warniełło
2022-01-03 20:04         ` Jonathan Corbet
2022-01-03 21:48           ` Tomasz Warniełło
2022-01-03 21:57             ` Jonathan Corbet
2022-01-03 23:56               ` Tomasz Warniełło
2022-01-03 20:40   ` Mauro Carvalho Chehab

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=87k0g336tl.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tomasz.warniello@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.