Code review comment for lp:~achiang/laika/configparser

On Mon, 11 Oct 2010 20:43:44 -0000
Alex Chiang <email address hidden> wrote:

> === modified file 'laika.py'
> --- laika.py 2010-08-17 19:00:08 +0000
> +++ laika.py 2010-10-11 20:43:43 +0000
> @@ -11,10 +11,10 @@
> # This program is distributed under the terms of the
> # GNU General Public License version 2.
>
> +import ConfigParser
> import datetime
> from optparse import OptionParser
> -import ConfigParser
> -import os, os.path
> +import os
> import re
> import sys
>
> @@ -22,7 +22,6 @@
>
> UTCNOW = datetime.datetime.utcnow()
>
> -
> class Report(object):
> '''An activity report for a specified Launchpad user.
>

PEP 8 wants this extra line here. If PEP 8 is important to you, you'll
want to put it back.

> @@ -137,24 +136,40 @@
> self.print_comments()
> self.print_reported()
>
> -def get_launchpad_username_from_bazaar():
> - bazaar_config = ConfigParser.ConfigParser()
> - bazaar_config.read(os.path.expanduser("~/.bazaar/bazaar.conf"))
> - return bazaar_config.get("DEFAULT", "launchpad_username")

This is a not-so-good way of getting the bazaar username... I guess
I'm happy it's going away.

> +def get_config(opts):
> + config = {}
> + FORMAT = "1.0"
> + laika_config = ConfigParser.ConfigParser()
> + laika_config.read(os.path.expanduser("~/.laikarc"))
> +
> + if laika_config.has_section(FORMAT) == False:
> + print "Error: unsupported configuration file format"
> + sys.exit()
> +
> + def default_user():
> + return os.getenv("USER")
> + def default_window():
> + return 8
> +
> + for opt in opts:
> + try:
> + config[opt] = laika_config.get(FORMAT, opt)
> + except ConfigParser.NoOptionError:
> + config[opt] = locals()["default_%s" % opt]()
> +
> + return config
>

How does this new function introduce the same functionality as
get_launchpad_username_from_bazaar - It seems like we're regressing
here from that functionality.

 review approve