Code review comment for lp:~jimbaker/pyjuju/charm-format-2

http://codereview.appspot.com/6308044/diff/10001/juju/charm/metadata.py
File juju/charm/metadata.py (right):

http://codereview.appspot.com/6308044/diff/10001/juju/charm/metadata.py#newcode239
juju/charm/metadata.py:239: if self.format not in (PYTHON_FORMAT,
YAML_FORMAT):
This seems to be conflating two different things, charm api versions and
there corresponding formats. ie. we could add version 3, with the
existing yaml format. there should be a mapping of format to version.
the charm isn't making a request for format, its making a version api
assertion.

http://codereview.appspot.com/6308044/diff/10001/juju/hooks/protocol.py
File juju/hooks/protocol.py (right):

http://codereview.appspot.com/6308044/diff/10001/juju/hooks/protocol.py#newcode220
juju/hooks/protocol.py:220: if invoker.charm_format == PYTHON_FORMAT:
instead of conditional logic everywhere in this file, this process would
be simpler/cleaner dispatched to a formatter object from lib/format as
an instance property.

http://codereview.appspot.com/6308044/diff/10001/juju/hooks/utils.py
File juju/hooks/utils.py (right):

http://codereview.appspot.com/6308044/diff/10001/juju/hooks/utils.py#newcode10
juju/hooks/utils.py:10: env_charm_format =
os.environ.get("_JUJU_CHARM_FORMAT")
this feels a bit icky, its an immutable property of a charm being passed
as a mutable parameter. the server side could return the format in its
response.

http://codereview.appspot.com/6308044/