Code review comment for lp:~bac/charms/precise/juju-gui/constraint-parsing-deux

Nice branch Brad, thank you!
LGTM with minor changes.

https://codereview.appspot.com/71230043/diff/1/HACKING.md
File HACKING.md (right):

https://codereview.appspot.com/71230043/diff/1/HACKING.md#newcode72
HACKING.md:72: Note that the test will not work using an LXC
environment. The test
Thank you for this! To be more explicit, we could say "Note that
functional tests will not work...".

https://codereview.appspot.com/71230043/diff/1/server/guiserver/bundles/utils.py
File server/guiserver/bundles/utils.py (left):

https://codereview.appspot.com/71230043/diff/1/server/guiserver/bundles/utils.py#oldcode40
server/guiserver/bundles/utils.py:40: # Define a sequence of allowed
constraints to be used in the process of
I am very happy that now we have a shared lib at least for this
constraints stuff.

https://codereview.appspot.com/71230043/diff/1/server/guiserver/bundles/utils.py
File server/guiserver/bundles/utils.py (right):

https://codereview.appspot.com/71230043/diff/1/server/guiserver/bundles/utils.py#newcode34
server/guiserver/bundles/utils.py:34: from charmworldlib.utils import
parse_constraints
Please keep those in alphabetical order.

https://codereview.appspot.com/71230043/diff/1/server/guiserver/bundles/utils.py#newcode160
server/guiserver/bundles/utils.py:160: - the bundle includes unsupported
constraints.
I see that parse_constraints raises a ValueError if constraints are not
valid. Maybe it's worth mentioning here too, or below, e.g.
- the bundle includes unsupported or invalid constraints.

We also might want to mention somewhere (perhaps in the requirements
file) that when updating the charmworldlib tgz, we should ensure
parse_constraints still only raises ValueErrors.

https://codereview.appspot.com/71230043/