Code review comment for lp:~lifeless/launchpad/databasefixture

Revision history for this message
Jonathan Lange (jml) wrote :

Hey Rob,

This looks good. Two concerns:

 1. The branch adds a new 'print' to pgsql.py. Is this leftover debugging? If so, delete it. If not, you're going to have to print more information, otherwise no one else will know what's going on.

 2. The way we add to the config here seems fragile: get the base layer, get two different configs, add the same thing twice. I wish there was a get_all_configs() or some kind of Composite object so that the knowledge about the -appserver business didn't have to be spread around so much.

I'm not going to block on the second point. Please address the first and then land.

jml

review: Approve

« Back to merge proposal