On 02/11/2010 06:31 PM, Jesus M. Rodriguez wrote:

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

Dmitri,

I have a few comments about the configuration work:

http://bit.ly/9XflrU

CandlpinConfiguration -

• why is it CandlepinConfiguration and not just Configuration? or Config :)

Gooood question. Changed to Config.

• why do we store the File object in CandlepinConfiguration? as opposed to storing just the filename, then in loadProperties() create a new File object there for loading.

Hmmm. I'll go with a personal preference here?

• I don't quite understand the configurationWithPrefix method?

returns all configuration entries with a given prefix. For example, given that you have:

a.b.c.d = value a.b.e.f = another b.c.d.e = yet another b.c.f.g = no more c.d.a.b = last one

and prefix b.c, configurationWithPrefix will return:

b.c.d.e = yet another b.c.f.g = no more

• why is there a special JPAConfiguration() class? (I'll respond more on this when I find the commit)

pls. see my previous comment. In short it used to parse persistence.xml

ConfigurationFileLoader -

• why store the File? could just make loadProperties() take in a file

• loadConfiguration() name seems redundant since it is the ConfigurationFileLoader(), I think load() would've sufficed.

good point. done.

• any reason we're converting Properties into HashMaps?

mostly to have type safety later when a TreeMap is constructed out of values in Properties.

So far I don't see how we'll use the CandlepinConfiguration. For instance, in spacewalk I'd commonly want to do: Config.get().getString("a.b.c.url"); or Config.get().getInt("a.b.timeoutvalue");

Maybe we're not to that point yet.

we're not using it at this point. It's going to be used to configure external (to candlepin) implementations of certain interfaces (packaged as guice modules) and JPA properties, such as db url, credentials, etc.

Please let me know if you more questions.

Cheers, -Dmitri