Even though this is a large branch I think it removes more code than it adds which is always a plus. I had some minor feedback but this LGTM.
I'd still rather Kapil get a chance to look this over but I think the general architectural issues that were present before (and which I didn't take into account either) are no longer present. 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; }); Don't you need to either use self here or pass this as context to the 'on' call? https://codereview.appspot.com/6733067/diff/1/app/models/charm.js#newcode84 app/models/charm.js:84: options.get_charm( Why the two naming styles on these_twoMethods? get_charm, loadByPath? 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> I thought you didn't need the #if when there is no content other than the var which can default to null, no? 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. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
