[anaconda][master][PATCH] Parse boot options before parsing CLI options (#1101341)
Martin Kolman
mkolman at redhat.com
Wed May 28 16:54:24 UTC 2014
On Wed, 2014-05-28 at 12:22 -0400, Anne Mulhern wrote:
> > > > 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.
So I'm removing "basically" locally to sync the NOTE: with the
docstring.
>
> - mulhern
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
More information about the anaconda-patches
mailing list