[anaconda][master][PATCH] Parse boot options before parsing CLI options (#1101341)

Anne Mulhern amulhern at redhat.com
Wed May 28 16:22:22 UTC 2014


> > >              if option.nargs == 0 and option.const is not None:
> > >                  # nargs == 0 & constr == True -> store_true
> > > @@ -224,11 +221,13 @@ class AnacondaArgumentParser(ArgumentParser):
> > >          :returns: an argparse Namespace instance
> > >          :rtype: Namespace
> > >          """
> > > -        # parse arguments (if any) and return the resulting namespace
> > > -        namespace = ArgumentParser.parse_args(self, args)
> > > -        # now parse boot options (if any) and modify the namespace
> > > accordingly
> > > -        namespace = self.parse_boot_cmdline(boot_cmdline, namespace)
> > > -        # and return the resulting namespace
> > > +        # parse boot options first
> > > +        namespace = self.parse_boot_cmdline(boot_cmdline)
> > > +        # parse CLI arguments (if any) and add them to the namespace
> > > +        # created from parsing boot options, overriding any options
> > > +        # with the same destination already present in the namespace
> > > +        # NOTE: this basically means that CLI options override boot
> > > options
> > 
> > I'ld leave out the note. When you say something "basically" does something,
> > you're implying that there are insidious, non-basic, ways in which
> > it does something else. If that's really true, but I don't think it is,
> > then the comment should really explain those instead.
> > 
> > To be honest, I'ld leave out all the comments here. You shouldn't feel the
> > need to work so hard just to save me from having to read the parse_args
> > documentation.
> Well, I think the way how the boot option and CLI option parsing works &
> interacts is a bit too tricky to "get" from just reading the source code
> and therefore I think the additional documentation is needed here.
> 
> > 
> > Surprisingly, the method comment already states that program cmdline
> > options
> > override boot cmdline arguments.
> > 

Sorry to be pedantic, but...

There is already a comment in the method header that states
"Program cmdline arguments will override boot cmdline arguments."
without any qualification.

Here's Google's take on 'basically':
Definition: in the most essential respects; fundamentally.
Usage: used to indicate that a statement summarizes the most important aspects, or gives a roughly accurate account, of a more complex situation
So, when I see 'basically' I wonder what actually is going on in this
more complex situation and why the comment doesn't help me
by explaining the tricky, complex part, which it hints at so disturbingly.

By qualifying its main statement with 'basically'
the note contradicts the method header by implying
that there is much more to the story than what seems apparent from
the source code (while the method header states that the code is doing
what it looks like its doing).

So, either the header comment or the note is misleading, and should be fixed.

- mulhern


More information about the anaconda-patches mailing list