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

Issue 8686047: Created a model controller with promises

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by jeff.pihach
Modified:
9 years, 7 months ago
Reviewers:
rharding, mp+161454, gary.poster
Visibility:
Public.

Description

Created a model controller with promises This branch takes the work from a number of previous branches and combined the work under a single ModelController class which will be used from now on to interact with the models and db to avoid having to pass the env and db into them. This also implements a sandbox fix from Gary. Individual unit tests will be coming in a follow-up but in the mean time all of this code is tested indirectly by the endpoints tests. https://code.launchpad.net/~hatch/juju-gui/model-control/+merge/161454 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 24

Patch Set 2 : Created a model controller with promises #

Unified diffs Side-by-side diffs Delta from patch set Stats (+322 lines, -177 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/app.js View 1 4 chunks +14 lines, -3 lines 0 comments Download
M app/config-prod.js View 1 chunk +1 line, -0 lines 0 comments Download
M app/models/charm.js View 1 chunk +0 lines, -2 lines 0 comments Download
A app/models/model-controller.js View 1 1 chunk +128 lines, -0 lines 0 comments Download
M app/models/models.js View 1 3 chunks +3 lines, -3 lines 0 comments Download
M app/modules-debug.js View 1 chunk +4 lines, -1 line 0 comments Download
M app/store/endpoints.js View 1 4 chunks +40 lines, -83 lines 0 comments Download
M app/store/env/fakebackend.js View 3 chunks +3 lines, -6 lines 0 comments Download
M app/store/env/sandbox.js View 2 chunks +49 lines, -27 lines 0 comments Download
M app/store/notifications.js View 1 chunk +4 lines, -1 line 0 comments Download
M test/test_app.js View 4 chunks +12 lines, -8 lines 0 comments Download
M test/test_endpoints.js View 1 7 chunks +58 lines, -38 lines 0 comments Download
M test/test_fakebackend.js View 3 chunks +4 lines, -5 lines 0 comments Download

Messages

Total messages: 6
jeff.pihach
Please take a look.
9 years, 7 months ago (2013-04-29 16:37:57 UTC) #1
gary.poster
LGTM with trivials. Thank you! Gary https://codereview.appspot.com/8686047/diff/1/app/models/model-controller.js File app/models/model-controller.js (right): https://codereview.appspot.com/8686047/diff/1/app/models/model-controller.js#newcode6 app/models/model-controller.js:6: models to avoid ...
9 years, 7 months ago (2013-04-29 18:22:03 UTC) #2
jeff.pihach
Thanks for the review! https://codereview.appspot.com/8686047/diff/1/app/models/model-controller.js File app/models/model-controller.js (right): https://codereview.appspot.com/8686047/diff/1/app/models/model-controller.js#newcode6 app/models/model-controller.js:6: models to avoid needing to ...
9 years, 7 months ago (2013-04-29 20:47:27 UTC) #3
rharding
LGTM with a couple of notes on confusion and take it or leave it clarity ...
9 years, 7 months ago (2013-04-30 12:36:59 UTC) #4
jeff.pihach
Thanks for the reviews! https://codereview.appspot.com/8686047/diff/1/app/models/model-controller.js File app/models/model-controller.js (right): https://codereview.appspot.com/8686047/diff/1/app/models/model-controller.js#newcode11 app/models/model-controller.js:11: var ModelController = Y.Base.create('model-controller', Y.Base, ...
9 years, 7 months ago (2013-04-30 15:49:38 UTC) #5
jeff.pihach
9 years, 7 months ago (2013-04-30 15:57:25 UTC) #6
*** Submitted:

Created a model controller with promises

This branch takes the work from a number of previous branches
and combined the work under a single ModelController class which
will be used from now on to interact with the models and db
to avoid having to pass the env and db into them. 

This also implements a sandbox fix from Gary.

Individual unit tests will be coming in a follow-up but in the
mean time all of this code is tested indirectly by the endpoints
tests.

R=gary.poster, rharding
CC=
https://codereview.appspot.com/8686047
Sign in to reply to this message.

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