linux-kernel - Re: [PATCH] scripts: kernel-doc: transform documentation into POD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87h7b8cfg0.fsf@meer.lwn.net>
Date:   Thu, 16 Dec 2021 16:12:15 -0700
From:   Jonathan Corbet <corbet@....net>
To:     Tomasz Warniełło <tomasz.warniello@...il.com>
Cc:     Tomasz Warniełło <tomasz.warniello@...il.com>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] scripts: kernel-doc: transform documentation into POD

Tomasz Warniełło <tomasz.warniello@...il.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...

> 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@...il.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@...hat.com>          ##
> -## Copyright (C) 2001  Simon Huggins                             ##
> -## Copyright (C) 2005-2012  Randy Dunlap                         ##
> -## Copyright (C) 2012  Dan Luedtke                               ##
> -## 								 ##
> -## #define enhancements by Armin Kuster <akuster@...sta.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@...th.li
> -
> -# 27/06/2001 -  Allowed whitespace after initial "/**" and
> -#               allowed comments before function declarations.
> -# -- Christian Kreibich <ck@...op.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@....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@...nsen.de>
> -
> -# 25/07/2012 - Added support for HTML5
> -# -- Dan Luedtke <mail@...rl.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