Code review comment for lp:~hloeung/charms/precise/apache2/ssl-security-options

Revision history for this message
Adam Israel (aisrael) wrote :

Hi Haw,

I've had a chance to review this merge proposal. As Charles mentioned, this will be a very nice addition to the apache2 charm. I'd also like to see some documentation added to the README to describe this new functionality, as well as unit tests against it.

There's one minor bug that could also use fixing. The Makefile needs to have `python-jinja2` added to the list of packages installed, or `make build` will fail in a clean environment.

I expect there'll be no problem promoting these changes to the store, once the above are addressed.

Thanks again for all of your work!

review: Needs Fixing

« Back to merge proposal