[lldb-dev] Auditing dotest's command line options

Tue Dec 8 23:18:11 PST 2015

I think it's a nice improvement.

Passing the options around via the argparse results (as I do in many
programs) makes it easier to unit test, but having configuration variables
all in a module makes it really simple to find and use everywhere without
having them as globals.

Thanks for cleaning that up, Zachary!

-Todd

On Tue, Dec 8, 2015 at 11:31 AM, Greg Clayton via lldb-dev <
lldb-dev at lists.llvm.org> wrote:

> Sounds good, looks good then.
>
> > On Dec 8, 2015, at 11:09 AM, Zachary Turner <zturner at google.com> wrote:
> >
> > One advantage of this approach is that it makes the options available to
> the entire test suite.  Even if we have no transferring going on, and we
> get argparse to return us a perfectly organized structure with everything
> in the right format, in order to make all the options accessible to the
> rest of the test suite, we still need to stick it in a global module
> somewhere.  And then you would write
> `configuration.options.test_categories`, whereas with this approach we just
> write `configuration.test_categories`.  It's a minor point, but I like the
> shorter member access personally.
> >
> > On Tue, Dec 8, 2015 at 11:07 AM Zachary Turner <zturner at google.com>
> wrote:
> > There's no way to avoid doing a transfer out of the options dictionary
> at some level, because it's not a straight transfer.  There's a ton of
> post-processing that gets done on the options dictionary in order to
> convert the raw options into a useful format.
> >
> > That might be solvable with more advanced use of argparse.  This
> approach does get rid of one level of option transfer though.  Because we
> would transfer
> > 1. From the class returned by argparse into the global
> > 2. From the global into the lldb module
> >
> > Now we only transfer from the argparse class into the `configuration`
> module, and everything else just uses that.
> >
> >
> > On Tue, Dec 8, 2015 at 10:52 AM Greg Clayton <gclayton at apple.com> wrote:
> > Do we not want to have an "options" global variable in this module that
> contains everything instead of having separate global variables in this
> file? The idea would be that you could assign directly when parsing
> arguments:
> >
> > (configuration.options, args) = parser.parse_args(sys.argv[1:])
> >
> > Its OK if we don't do this, but this is what I was originally thinking.
> Then we don't need to do any transfer out of the options dictionary that is
> returned by the option parser. The drawback with this approach is the
> "configuration.options" would probably need to be initialized in case
> someone tries to access the "configuration.options" without first parsing
> arguments. So in that respect the global approach is nicer.
> >
> > Greg
> >
> > > On Dec 8, 2015, at 10:45 AM, Zachary Turner <zturner at google.com>
> wrote:
> > >
> > > Hi Greg,
> > >
> > > Take a look at dotest.py next time you get some free time and let me
> know what you think.  There should be no more globals.  Everything that
> used to be a global is now stored in its own module `configuration.py`, and
> everything in `configuration.py` can be referenced from everywhere in the
> entire test suite.
> > >
> > > On Fri, Nov 20, 2015 at 10:34 AM Greg Clayton <gclayton at apple.com>
> wrote:
> > > Zach, I would also like to get rid of all global variables in the
> process of this change. The history goes like this: a long time ago someone
> wrote the initial dotest.py and parsed the options manually and stored
> results in global variables. Later, someone converted the options over to
> use a python library to parse the options, but we mostly copied the options
> from the options dictionary over into the globals and still use the globals
> all over the code. It would be great if we had at most one global variable
> that is something like "g_options" and anyone that was using any global
> variables will switch over to use the "g_options.XXXX" instead. Then we
> don't have to make copies and we can let the g_options contain all settings
> that are required.
> > >
> > > > On Nov 18, 2015, at 2:32 PM, Zachary Turner via lldb-dev <
> lldb-dev at lists.llvm.org> wrote:
> > > >
> > > > I would like to do a complete audit of dotest's command line
> options, find out who's using what, and then potentially delete anything
> that isn't being used.  There's a mess of command line options in use, to
> the point that it's often hard to find free letters to use for new options.
> > > >
> > > > I created this spreadsheet with a complete list of command line
> options, their descriptions, and a place for people to enter what options
> they're using or do not want to be deleted.
> > > >
> > > >
> https://docs.google.com/spreadsheets/d/1wkxAY7l0_cJOHhhsSlh3aKKlQShlX1D7X1Dn8kpqxy4/edit?usp=sharing
> > > >
> > > > If someone has already written YES in the box that indicates they
> need the option, please don't overwrite it.  If you write YES in a box,
> please provide at least a small rationale for why this option is useful to
> you.  Feel free to add additional rationale if someone has already added
> some rationale.
> > > >
> > > > I'm going to have a couple days in mid-December and do this cleanup,
> so I'd like to get a solid picture of what options are not needed before
> then.  After people have had some time to look over this, I'll go through
> the results and decide what to do with each one, and then send out another
> email with a proposed action column for each command line option.
> > > >
> > > > Please do take the time to have a look at this, because any option
> that doesn't have a YES in it after a couple of weeks I'm going to assume
> is a candidate for deletion.
> > > >
> > > >
> > > > _______________________________________________
> > > > lldb-dev mailing list
> > > > lldb-dev at lists.llvm.org
> > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
> > >
> >
>
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>

-- 
-Todd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20151208/647f2fc2/attachment.html>