Code review comment for lp://staging/~allanlesage/qlbr/fab-deployment

Revision history for this message
Max Brustkern (nuclearbob) wrote :

> So I'm not able to get the export to work out of the box, going to leave as-is
> and pick that up in a bug if you don't mind Max?

Works for me, we can make a bug or add it to the blueprint.

On merge line 13, I like to have sudo before all recommended apt-get commands. That makes me feel more ubuntu-y. I also wonder if maybe adding the $ prompt to other command recommendations is a good idea. We can do that in another merge, though.

Do we want to mention something about the REPLACE_ME stuff in deployment/production_settings.py, staging_settings.py, and testing_settings.py?

Have you tested deploy_crontab? It seems like crontab_filename and crontab_filepath maybe ought to be the same? I also wonder if it would make sense to have a default available so we can automatically deploy the recommended one, but it seems like the most recent commit did away with that, so I'm guessing there's a reason.

On line 529 of the merge, is lp:helipad what we want, or is that left over from a previous project?

review: Needs Information

« Back to merge proposal