Code review comment for lp:~thumper/charms/trusty/python-django/support-1.7

Revision history for this message
Tim Penhey (thumper) wrote :

On 23/05/15 07:54, Charles Butler wrote:
> Review: Approve
>
> Tim,
>
> Thank you for this contribution! It's great to see the python-django charm getting some love as its one of our few framework charms. I feel like your contributions have made a great impact in cleaning up some of the nomenclature and functionality of the charm for future contributors.
>
> I deployed the charm and gave the charm a deploy test along with stressing the existing charm before pulling in your changes.
>
> There is a minor linting issue that cropped up on this contribution:
>
> FAIL: python-django::make lint
> [/usr/bin/make -s lint exit 2]
> Lint check (flake8)
> hooks/hooks.py:44:19: E128 continuation line under-indented for visual indent
> hooks/hooks.py:251:22: F821 undefined name 'semantic_version'
> hooks/hooks.py:254:40: F821 undefined name 'semantic_version'
> make: *** [lint] Error 1
>
> If you could get those cleaned up in a subsequent patch, that would be +1 from me.
>
>
> Thank you for taking the time to submit this fix for the charm store. We appreciate your work. I've merged this branch and it should be available in the charm store after the next ingestion. I look forward to the forthcoming updates to the python-django ecosystem from you :)
>
>
> If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

I think we need to look at the process for checking that the charm works.

The lint issues identified above show that the code is clearly broken.
I have submitted another merge proposal that not only fixes this lint,
but adds tests to make sure it works as expected.

Tim

« Back to merge proposal