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

Jan Tulak
2016-04-08T10:31:08Z

[-- Attachment #1.1: Type: text/plain, Size: 2377 bytes --]

On Thu, Apr 7, 2016 at 7:25 PM, Eric Sandeen <sandeen@sandeen.net> wrote:

>
>
> "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.
>
> ​Yeah, I should reformulate it, I think. The meaning is that it won't
matter what the defaults are now, or will be in the future. E.g., if you
had a script creating a fs without crc before, when it was disabled by
default, and we changed the default, you are now creating with the crc. But
if you give it -m crc=0, then no matter what the default is, you have it
always disabled.

How about changing the line to "Boolean options allows for optional
argument of value 0 or 1, to explicitly disable or enable the
functionality," and dropping the forward-compatible part?

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

​For options inside of -m, -d and such, yes. Top-level flags, that is -f,
-q, -N, -K and -V, are still only flags, but these don't change the FS
attributes. They are something different from the other. Still, I wonder
whether they should accept [0|1] too...

​

>
> > @@ -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.
>
> Done​

> Thanks,
> -Eric
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>

​Thanks,
Jan​

--
Jan Tulak
jtulak@redhat.com / jan@tulak.me

[-- Attachment #1.2: Type: text/html, Size: 4952 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]