yellow team mailing list archive

[Gary Poster <gary.poster@xxxxxxxxxxxxx>]

Reviewers: mp+130464_code.launchpad.net,

Message:
Please take a look.

Description:
Separate environment and store charms

On discussion with Kapil, a significant problem from a previous
refactoring of mine became clear.  Charm ids are only reliably unique
within a given context, such as juju environment vs charm store.
Therefore, charms from the two sources need to be stored separately.

We discussed some approaches.  He suggested storing environment charms
in the database and store charms in the browser, and I liked that idea.
He also requested that I factor out the charm model code into a separate
file, even while keeping it in the juju.models package.  Finally, he
suggested that charm ids should always include revisions in order to
guarantee uniqueness, and to make it possible to consider whether charms
from the different sources are identical.

I wrote a long explanation of this in the app/models/charms.js file,
including additional considerations.

The charm store needed to send the revision itself, which Kapil changed
it to do.

I ended up also factoring out a charm store object, with the ability to
get the data from a search, organized as we like it; and to get the data
for a specific charm.  A nice fall out from the tests of this code is
that it exposed some pre-existing problems with sorting code, which I
addressed.

The change is quite large because it touches so many of the files.  On
the bright side, many of the test changes are mechanical, and much of
the code is moved and only slightly refactored and changed from other
sources, so deletions and moves account for much of the churn.  That
said, it is still a large branch, for which I apologize.

Thanks,

Gary

https://code.launchpad.net/~gary/juju-gui/charmdivision/+merge/130464

(do not edit description out of merge proposal)


Please review this at https://codereview.appspot.com/6749046/

Affected files:
   M .jshintrc
   A [revision details]
   M app/app.js
   A app/models/charm.js
   M app/models/models.js
   M app/modules.js
   M app/store/charm.js
   M app/templates/charm-search-result.handlebars
   M app/views/charm-search.js
   M app/views/charm.js
   A test/data/search_results.json
   A test/data/series_search_results.json
   M test/index.html
   M test/test_app.js
   M test/test_charm_configuration.js
   M test/test_charm_search.js
   A test/test_charm_store.js
   M test/test_charm_view.js
   M test/test_model.js
   M test/test_service_config_view.js
   M test/test_unit_view.js



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