Code review comment for lp:~fwereade/pyjuju/isolate-formula-revisions

This looks fantastic. Just a few trivials, and let's try to get a +1 from Kapil.

[5]

+ revision = self.get_revision()
+ if revision is None:
+ raise CharmError(path, "has no revision")

Hmmm.. both implementations are doing this, so maybe the common
get_revision function itself could take care of it if neither
options have a valid revision.

[6]

+ revision = self.get_revision()
+ if revision is None:
+ raise CharmError(path, "has no revision")
+ if self._get_revision_file_content() is None:
+ self.set_revision(revision)

_get_revision_file_content is being called necessarily and at
least twice every time. Plus when the revision is actually
wanted outside of the implementation.

Maybe we should cache in a private attibute instead?

[7]

+ def set_revision(self, revision):
+ with open(os.path.join(self.path, "revision"), "w") as f:
+ f.write(str(revision))

A "\n" at the end would be nice to deal with it from the shell.