Re: [PATCH 08/19] mkfs: getbool is redundant

From: Eric Sandeen <sandeen@sandeen.net>
To: xfs@oss.sgi.com
Subject: Re: [PATCH 08/19] mkfs: getbool is redundant
Date: Thu, 7 Apr 2016 12:25:46 -0500	[thread overview]
Message-ID: <5706981A.9090904@sandeen.net> (raw)
In-Reply-To: <1458818136-56043-9-git-send-email-jtulak@redhat.com>

On 3/24/16 6:15 AM, jtulak@redhat.com wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> CHANGELOG:
> o Add description of a new member of opt_params
> o Fix in the subopts tables
> o All boolean subopts should use getnum_checked
> o Edit man8 page where it missed the information about optional value of a flag
> 
> getbool() can be replaced with getnum_checked with appropriate
> min/max values set for the boolean variables. Make boolean
> arguments consistent - all accept 0 or 1 value now.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
>  man/man8/mkfs.xfs.8 |  11 ++-
>  mkfs/xfs_mkfs.c     | 223 ++++++++++++++++++++++++++++++++++++----------------
>  2 files changed, 164 insertions(+), 70 deletions(-)
> 
> diff --git a/man/man8/mkfs.xfs.8 b/man/man8/mkfs.xfs.8
> index 8b068ef..5a2a20a 100644
> --- a/man/man8/mkfs.xfs.8
> +++ b/man/man8/mkfs.xfs.8
> @@ -118,6 +118,9 @@ option or the
>  option first needs to be added to the command line.
>  Failure to specify the size of the units will result in illegal value errors
>  when parameters are quantified in those units.
> +.PP
> +Many options allows for optional argument of value 0 or 1, to explicitly

"Many options allow for an optional argument of 0 or 1, ..."

> +disable or enable the functionality, in a forward-compatible syntax.

What does "forward-compatible syntax" mean?  I'm not sure that clarifies
anything for the reader.

Otherwise this looks ok to me; Dave explained that it is intentional to
make every single option accept a value, whether it is now
boolean or a numeric value, so there is no such thing as a bare "--flag"
anymore; such flags are always "--flag [0|1]" now, right?

> @@ -312,10 +315,12 @@ and
>  .B swidth
>  values.
>  .TP
> -.BI noalign
> -This option disables automatic geometry detection and creates the filesystem
> +.BI noalign[= value ]
> +If the
> +.I value
> +is 1, this option disables automatic geometry detection and creates the filesystem
>  without stripe geometry alignment even if the underlying storage device provides
> -this information.
> +this information. 0 disables this option, that is, enables automatic detection.
>  .RE
>  .TP
>  .B \-f

The realtime section of the manpage needs this same fixup now too, I guess.

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs