change charm store data structures

app/models/charm.js

 'use strict';
 
 YUI.add('juju-charm-models', function(Y) {
 
 
   var models = Y.namespace('juju.models');
 
   // Charms, once instantiated and loaded with data from their respective
   // sources, are immutable and read-only. This reflects the reality of how we
   // interact with them.
@@ skipping 24 matching lines @@
   // "charm.load(charm_store, optionalCallback)".  The get_charm method must
   // either callback using the default YUI approach for this code, a boolean
   // indicating failure, and a result; or it must return what the env version
   // does: an object with a "result" object containing the charm data, or an
   // object with an "err" attribute.
 
   // In both cases, environment charms and charm store charms, a charm's
   // "loaded" attribute is set to true once it has all the data from its
   // environment.
 
-  var charm_id_re = /^(?:(\w+):)?(?:~(\S+)\/)?(\w+)\/(\S+?)-(\d+)$/,
-      id_elements = ['scheme', 'owner', 'series', 'package_name', 'revision'],
+  var charmIdRe = /^(?:(\w+):)?(?:~(\S+)\/)?(\w+)\/(\S+?)-(\d+)$/,
+      idElements = ['scheme', 'owner', 'series', 'package_name', 'revision'],
       Charm = Y.Base.create('charm', Y.Model, [], {
         initializer: function() {
           var id = this.get('id'),
-              parts = id && charm_id_re.exec(id),
+              parts = id && charmIdRe.exec(id),
               self = this;
           if (!Y.Lang.isValue(id) || !parts) {
             throw 'Developers must initialize charms with a well-formed id.';
           }
           this.loaded = false;
           this.on('load', function() { this.loaded = true; });

[benjamin.saller, 2012/10/26 07:56:19]

Don't you need to either use self here or pass this as context to the 'on' call?

[gary.poster, 2012/10/26 12:11:21]

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.

           parts.shift();
           Y.each(
-              Y.Array.zip(id_elements, parts),
+              Y.Array.zip(idElements, parts),
               function(pair) { self.set(pair[0], pair[1]); });
           // full_name
           var tmp = [this.get('series'), this.get('package_name')],
               owner = this.get('owner');
           if (owner) {
             tmp.unshift('~' + owner);
           }
           this.set('full_name', tmp.join('/'));
           // charm_store_path
           this.set(
               'charm_store_path',
               [(owner ? '~' + owner : 'charms'),
                this.get('series'),
                (this.get('package_name') + '-' + this.get('revision')),
                'json'
               ].join('/'));
         },
         sync: function(action, options, callback) {
           if (action !== 'read') {
             throw (
                 'Only use the "read" action; "' + action + '" not supported.');
           }
           if (Y.Lang.isValue(options.get_charm)) {
             // This is an env.
             options.get_charm(

[benjamin.saller, 2012/10/26 07:56:19]

Why the two naming styles on these_twoMethods? get_charm, loadByPath?

[gary.poster, 2012/10/26 12:11:21]

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.

                 this.get('id'),
                 function(response) {
                   if (response.err) {
                     callback(true, response);
                   } else if (response.result) {
                     callback(false, response.result);
                   } else {
                     // What's going on?  This does not look like either of our
                     // expected signatures.  Declare a loading error.
                     callback(true, response);
@@ skipping 51 matching lines @@
                 other.get('package_name'))) ||
                 (owner ? owner.localeCompare(otherOwner) : 0) ||
                 (this.get('revision') - other.get('revision')));
           }
         }
       }, {
         ATTRS: {
           id: {
             writeOnce: true,
             validator: function(val) {
-              return Y.Lang.isString(val) && !!charm_id_re.exec(val);
+              return Y.Lang.isString(val) && !!charmIdRe.exec(val);
             }
           },
           bzr_branch: {writeOnce: true},
           charm_store_path: {writeOnce: true},
           config: {writeOnce: true},
           description: {writeOnce: true},
           full_name: {writeOnce: true},
           is_subordinate: {writeOnce: true},
           last_change: {
             writeOnce: true,
@@ skipping 47 matching lines @@
     ATTRS: {}
   });
   models.CharmList = CharmList;
 
 }, '0.1.0', {
   requires: [
     'model',
     'model-list'
   ]
 });