Code review comment for lp:~patrick-hetu/charms/precise/python-django/charmhelpers

Revision history for this message
Charles Butler (lazypower) wrote :

Patrick,

Wow, this was a pretty significant change! I've reviewed the incoming changes and have the following notes:

This is an interesting mix of Ansible and plane ole python hook code. What was the prompt to move the cloning bits to Ansible vs just using the python hooks for installation? Not a nack, just really curious about the architecture design pattern.

Knitpicks:
There is a missing 00-setup script for the included test, and missing dependencies prevented the test execution from completing successfully on first run.

The tests also report errors as is, and are a prime candidate for being rewritten in amulet, or refactored as is so they pass.

Aside from these comments, I see no major blockers.

Thanks again! +1 LGTM

« Back to merge proposal