Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2416)

Issue 12608043: Removes Charm extension from BrowserCharm

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by j.c.sackett
Modified:
10 years, 9 months ago
Reviewers:
rharding, matthew.scott, mp+179005
Visibility:
Public.

Description

Removes Charm extension from BrowserCharm In order to be able to safely remove Charm, BrowserCharm cannot extend it. -BrowserCharm now extends Model -BrowserCharm now supports the initialization functions that Charm does (e.g. setting attr from the provided ID) -BrowserCharm now has the same `load` pipeline functions (e.g. `sync`) -Tests have been updated or added to demonstrate new support -Drive by test cleanup--there were many tests that checked the same exact things; I've merged these to mitigate the damage from some of the test repetition I've had to temporarily add. https://code.launchpad.net/~jcsackett/juju-gui/remove-charm-extension-in-browser-charm/+merge/179005 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1

Patch Set 2 : Removes Charm extension from BrowserCharm #

Unified diffs Side-by-side diffs Delta from patch set Stats (+377 lines, -91 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/models/charm.js View 1 4 chunks +112 lines, -5 lines 0 comments Download
M test/test_model.js View 6 chunks +263 lines, -86 lines 0 comments Download

Messages

Total messages: 4
j.c.sackett
Please take a look.
10 years, 9 months ago (2013-08-07 15:31:13 UTC) #1
rharding
LGTM getting closer! https://codereview.appspot.com/12608043/diff/1/app/models/charm.js File app/models/charm.js (right): https://codereview.appspot.com/12608043/diff/1/app/models/charm.js#newcode540 app/models/charm.js:540: if (data.config && data.config.options && ! ...
10 years, 9 months ago (2013-08-07 16:43:48 UTC) #2
matthew.scott
LGTM, thanks!
10 years, 9 months ago (2013-08-07 17:14:22 UTC) #3
j.c.sackett
10 years, 9 months ago (2013-08-07 21:34:33 UTC) #4
*** Submitted:

Removes Charm extension from BrowserCharm

In order to be able to safely remove Charm, BrowserCharm cannot extend it.
-BrowserCharm now extends Model
-BrowserCharm now supports the initialization functions that Charm does (e.g.
setting attr from the provided ID)
-BrowserCharm now has the same `load` pipeline functions (e.g. `sync`)
-Tests have been updated or added to demonstrate new support
-Drive by test cleanup--there were many tests that checked the same exact
things; I've merged these to mitigate the damage from some of the test
repetition I've had to temporarily add.

R=rharding, matthew.scott
CC=
https://codereview.appspot.com/12608043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b