mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jonathan Corbet <corbet@lwn.net>
To: "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: Thu, 16 Dec 2021 16:12:15 -0700	[thread overview]
Message-ID: <87h7b8cfg0.fsf@meer.lwn.net> (raw)
In-Reply-To: <20211209225549.69010-1-tomasz.warniello@gmail.com>

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...

> 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

  reply	other threads:[~2021-12-16 23:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-09 22:55 Tomasz Warniełło
2021-12-16 23:12 ` Jonathan Corbet [this message]
2021-12-17  9:43   ` Jani Nikula
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=87h7b8cfg0.fsf@meer.lwn.net \
    --to=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®