Code review comment for lp:~rharding/juju-gui/bundle-counts

Revision history for this message
Richard Harding (rharding) wrote :

Reviewer comments

https://codereview.appspot.com/30210044/diff/20001/app/models/bundle.js
File app/models/bundle.js (right):

https://codereview.appspot.com/30210044/diff/20001/app/models/bundle.js#newcode57
app/models/bundle.js:57: this.set('recent_download_count',
cfg.downloads_in_past_30_days);
the api is downloads_in_past_30_days but we've moved that before from 90
and such. So we map it to a more generic "recent" attr.

https://codereview.appspot.com/30210044/diff/20001/app/models/charm.js
File app/models/charm.js (right):

https://codereview.appspot.com/30210044/diff/20001/app/models/charm.js#newcode476
app/models/charm.js:476: downloads: {
drive-by, not sure how this attr was missing from before.

https://codereview.appspot.com/30210044/diff/20001/app/templates/bundle-token.handlebars
File app/templates/bundle-token.handlebars (right):

https://codereview.appspot.com/30210044/diff/20001/app/templates/bundle-token.handlebars#newcode26
app/templates/bundle-token.handlebars:26: <div>Deployed {{downloads}}
{{pluralize 'time' downloads}}</div>
per UX, make the first item in the metadata list.

https://codereview.appspot.com/30210044/diff/20001/test/data/browserbundle.json
File test/data/browserbundle.json (right):

https://codereview.appspot.com/30210044/diff/20001/test/data/browserbundle.json#newcode1
test/data/browserbundle.json:1: {
IGNORE ME: Updated test file due to api changes. It's a mechanical
change of downloading new api result.

https://codereview.appspot.com/30210044/diff/20001/test/test_model_bundle.js
File test/test_model_bundle.js (right):

https://codereview.appspot.com/30210044/diff/20001/test/test_model_bundle.js#newcode184
test/test_model_bundle.js:184: assert.lengthOf(commits, 5);
the rest of these are making the old tests match the updated api
response.

https://codereview.appspot.com/30210044/diff/20001/test/test_model_bundle.js#newcode195
test/test_model_bundle.js:195: // Manually added a second author into
the test data from the server.
this was manually tweaked in that api response so noted it here for
future test failures.

https://codereview.appspot.com/30210044/

« Back to merge proposal