Code review comment for lp:~frankban/charms/precise/juju-gui/support-firewall

Wow! Great tests, and very nice improvement. Code LGTM. I will check
with Rick as to which of us will QA.

Thank you,

Gary

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py
File hooks/backend.py (right):

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode116
hooks/backend.py:116: debs = ('curl',)
Does this belong somewhere else now? Or can it be deleted?

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode229
hooks/backend.py:229: If reverse is True, the method is called on the
reversed list of objects.
Delete this line? (functionality is gone)

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode310
hooks/backend.py:310: """Execute the charm starting steps."""
A subtle thing, but any one of these would read a bit better to me, as
ranked from my personal highest to lowest preference.

Execute the charm's "start" steps.
Execute the charm's starting steps.
Execute the charm-starting steps.

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode314
hooks/backend.py:314: """Execute the charm stopping steps."""
Perhaps the following?
----
Execute the charm's "stop" steps.

Iterates through the mixins in reverse order.

https://codereview.appspot.com/14433049/diff/1/hooks/backend.py#newcode318
hooks/backend.py:318: """Execute the charm removal steps."""
WDYT?
---
Execute the charm removal steps.

Iterates through the mixins in reverse order.

https://codereview.appspot.com/14433049/diff/1/hooks/shelltoolbox.py
File hooks/shelltoolbox.py (right):

https://codereview.appspot.com/14433049/diff/1/hooks/shelltoolbox.py#newcode3
hooks/shelltoolbox.py:3: # This file is part of python-shell-toolbox.
I included comments about the source in hooks/charmhelpers.py. Check
what I did there. If you like it, include it here; otherwise, feel free
to ignore it.

https://codereview.appspot.com/14433049/