Code review comment for lp:~johnsca/charms/trusty/cf-hm9000/services

Revision history for this message
Cory Johns (johnsca) wrote :

On 2014/06/02 18:29:44, benjamin.saller wrote:

https://codereview.appspot.com/104820043/diff/1/hooks/config.py#newcode24
> hooks/config.py:24: 'required_data': [contexts.NatsRelation(),
> What do you think about a top level define of this list that we reuse,
something
> like:

> required_data: hm_contexts

> where hm_contexts is defined above the service block.

+1 I definitely should have done this to begin with.

https://codereview.appspot.com/104820043/diff/1/templates/cf-hm9k-analyzer.conf#newcode2
> templates/cf-hm9k-analyzer.conf:2: author "Cory Johns
> <mailto:<email address hidden>>"
> This is an existing issue, but I'd like for all the Authors in all the
projects
> to point to the projects list rather than a bunch of individuals. The
charms
> maintainer fields as well. We should make a card for this and switch
them at
> once.

Easy enough to do in this charm during this review, then we can fix the
existing ones in a batch.

https://codereview.appspot.com/104820043/diff/1/templates/hm9000.json#newcode13
> templates/hm9000.json:13: "store_urls":
> ["http://{{etcd['hostname']}}:{{etcd['port']}}"],
> This could easily be a list, no? I think we'll want to handle it that
way (via
> iteration), but for now this is fine.

This raises an issue with how I implemented multiple units in the
framework, which I will address now.

https://codereview.appspot.com/104820043/

« Back to merge proposal