Code review comment for lp:~benji/charmworld/bundle-deploy-metric-api

Revision history for this message
Benji York (benji) wrote :

I forgot to publish my draft comments on Friday. I know everyone has
been waiting patiently for them.

https://codereview.appspot.com/23740043/diff/1/charmworld/testing/factory.py
File charmworld/testing/factory.py (right):

https://codereview.appspot.com/23740043/diff/1/charmworld/testing/factory.py#newcode365
charmworld/testing/factory.py:365: makeBundle = make_bundle
On 2013/11/08 19:03:29, bac wrote:
> You sure you'd not like to just fix them?

I'd like to... but I figured a mechanical branch later would be better.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__.py
File charmworld/views/api/__init__.py (right):

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__.py#newcode511
charmworld/views/api/__init__.py:511: bundle_metric_types =
('deployments')
Good eye. Fixed.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/api/__init__.py#newcode521
charmworld/views/api/__init__.py:521: if metric_name not in
self.bundle_metric_types:
On 2013/11/08 19:03:29, bac wrote:
> Huh, it works anyway.

Well, I learned something new today.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_metrics_api.py
File charmworld/views/tests/test_metrics_api.py (right):

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_metrics_api.py#newcode2
charmworld/views/tests/test_metrics_api.py:2:
On 2013/11/08 19:03:29, bac wrote:
> del

Done.

https://codereview.appspot.com/23740043/diff/1/charmworld/views/tests/test_metrics_api.py#newcode20
charmworld/views/tests/test_metrics_api.py:20:
On 2013/11/08 19:03:29, bac wrote:
> Thank you for not piling onto the other test files.

Yeah, we need to start breaking those files apart. A few are way over
the weight limit.

https://codereview.appspot.com/23740043/

« Back to merge proposal