Code review comment for lp:~abentley/charms/precise/charmworld/auto-production

Revision history for this message
Aaron Bentley (abentley) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 13-01-29 11:11 AM, <email address hidden> wrote:
> I wonder a little bit if we can reuse the configparser merging code
> abel has in charmworld/lib/config

The code in charmworld/lib/config doesn't do the merging, it delegates
that to ConfigParser.read, the same as mine does.

The bulk of charmworld/lib/config.py is concerned with emitting
warnings when sections and options are used in the override that are
not present in the default. However, this does not indicate a
problem. For example, the "logger_exc_logger" section in
production_overrides.ini is entirely legitimate, and it would be
inappropriate in the default, because the default logging
configuration is different. So that code is not useful in this context.

If I wanted to use Abel's script anyhow, it would need to take 3
inputs: defaults, production overrides, local configuration. Since it
doesn't, it's not useful here. If it did, I'd still need a Python
script. It would use ConfigParser to write out a temporary config for
local configuration.

> Should there be some documentation around to help aid other people
> working on the charm on how the parts fit together?

To me, the ideal is that the code is clear enough that documentation
is superfluous. I'm not sure whether we've reached that here.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iEYEARECAAYFAlEH+NgACgkQ0F+nu1YWqI2TVwCghD7+Rvvcornf81vhFAJZIlwR
TicAn1rcTH29iQ5IRO8yOfwfKjzD63iD
=8cOq
-----END PGP SIGNATURE-----

« Back to merge proposal