yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

*** Submitted:

change charm store data structures

This change is hopefully the last round of changes, at least for a long
while, to the underlying charm store infrastructure.  It is more deletes
than additions, and changes the code to take advantage of the changes
Kapil made to the charm store.

The sorting code is simplified yet again.

R=benjamin.saller
CC=
https://codereview.appspot.com/6733067



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

https://codereview.appspot.com/6733067/diff/1/app/models/charm.js#newcode56
app/models/charm.js:56: this.on('load', function() { this.loaded = true;
});
On 2012/10/26 07:56:19, benjamin.saller wrote:
> Don't you need to either use self here or pass this as context to the
'on' call?
No, because the listener is on "this" so the context is what I want.  I
already have a test that verifies, and I doublechecked in the chromium
debugger as well.

https://codereview.appspot.com/6733067/diff/1/app/models/charm.js#newcode84
app/models/charm.js:84: options.get_charm(
On 2012/10/26 07:56:19, benjamin.saller wrote:
> Why the two naming styles on these_twoMethods? get_charm, loadByPath?
As we discussed, it's because we haven't standardized one way or the
other across all our files.  Within a file, we are consistent.  This
uses get_charm, from the older env js, and loadByPath, from the newer
charm store js.  I got your agreement in person that this is fine.
OTOH I switched charmIDRe and idElements in this file, based on your
reminder.

https://codereview.appspot.com/6733067/diff/1/app/templates/charm-search-result.handlebars
File app/templates/charm-search-result.handlebars (right):

https://codereview.appspot.com/6733067/diff/1/app/templates/charm-search-result.handlebars#newcode13
app/templates/charm-search-result.handlebars:13: {{#if
owner}}{{owner}}/{{/if}}{{package_name}}</a>
On 2012/10/26 07:56:19, benjamin.saller wrote:
> I thought you didn't need the #if when there is no content other than
the var
> which can default to null, no?
Yes, but I have the slash after the owner.

https://codereview.appspot.com/6733067/

-- 
https://code.launchpad.net/~gary/juju-gui/simplifycharmstore/+merge/131086
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/simplifycharmstore into lp:juju-gui.