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

Issue 7381055: Created app subapp extension

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

Description

Created app subapp extension https://code.launchpad.net/~hatch/juju-gui/1130787-subapp-app-extension/+merge/150414 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 29

Patch Set 2 : Created app subapp extension #

Total comments: 10

Patch Set 3 : Created app subapp extension #

Patch Set 4 : Created app subapp extension #

Patch Set 5 : Created app subapp extension #

Total comments: 22

Patch Set 6 : Created app subapp extension #

Total comments: 2

Patch Set 7 : Created app subapp extension #

Patch Set 8 : Created app subapp extension #

Patch Set 9 : Created app subapp extension #

Patch Set 10 : Created app subapp extension #

Patch Set 11 : Created app subapp extension #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -3 lines) Patch
A [revision details] View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -0 lines 0 comments Download
M app/app.js View 1 2 3 4 5 6 7 8 9 10 3 chunks +14 lines, -3 lines 0 comments Download

Messages

Total messages: 21
jeff.pihach
Please take a look.
7 years, 9 months ago (2013-02-25 19:16:41 UTC) #1
jeff.pihach
Please take a look.
7 years, 9 months ago (2013-02-25 19:46:20 UTC) #2
bcsaller
Thanks for the branch. I have some minor comments below but there are still things ...
7 years, 9 months ago (2013-02-25 20:12:13 UTC) #3
jeff.pihach
Thanks for the review - I'll make the changes and resubmit https://codereview.appspot.com/7381055/diff/3001/app/assets/javascripts/app-subapp-extension.js File app/assets/javascripts/app-subapp-extension.js (right): ...
7 years, 9 months ago (2013-02-25 20:22:36 UTC) #4
jeff.pihach
Please take a look.
7 years, 9 months ago (2013-02-25 21:02:52 UTC) #5
gary.poster
Hi Jeff. I like where this is going. As I said on IRC, my biggest ...
7 years, 9 months ago (2013-02-25 21:20:05 UTC) #6
jeff.pihach
Thanks a bunch for your comments I'll get on them right away! I only have ...
7 years, 9 months ago (2013-02-25 21:46:59 UTC) #7
gary.poster
Hey Jeff. I made replies to some of your comments. https://codereview.appspot.com/7381055/diff/1/app/assets/javascripts/app-subapp-extension.js File app/assets/javascripts/app-subapp-extension.js (right): https://codereview.appspot.com/7381055/diff/1/app/assets/javascripts/app-subapp-extension.js#newcode42 ...
7 years, 9 months ago (2013-02-25 21:59:00 UTC) #8
bcsaller
https://codereview.appspot.com/7381055/diff/1/app/assets/javascripts/app-subapp-extension.js File app/assets/javascripts/app-subapp-extension.js (right): https://codereview.appspot.com/7381055/diff/1/app/assets/javascripts/app-subapp-extension.js#newcode134 app/assets/javascripts/app-subapp-extension.js:134: if (value.path !== '*') { On 2013/02/25 21:59:00, gary.poster ...
7 years, 9 months ago (2013-02-26 02:00:54 UTC) #9
jeff.pihach
Please take a look.
7 years, 9 months ago (2013-02-26 17:13:13 UTC) #10
jeff.pihach
Please take a look.
7 years, 9 months ago (2013-02-26 19:49:34 UTC) #11
bcsaller
Thanks for the continued work on this branch. Personally I think we should switch to ...
7 years, 9 months ago (2013-02-26 22:00:56 UTC) #12
gary.poster
LGTM with trivials. Hi Jeff. I have a lot of comments and suggestions and thoughts. ...
7 years, 9 months ago (2013-02-26 22:07:25 UTC) #13
jeff.pihach
Thanks for the reviews! Please see the attached comments https://codereview.appspot.com/7381055/diff/16001/app/assets/javascripts/app-subapp-extension.js File app/assets/javascripts/app-subapp-extension.js (right): https://codereview.appspot.com/7381055/diff/16001/app/assets/javascripts/app-subapp-extension.js#newcode47 app/assets/javascripts/app-subapp-extension.js:47: ...
7 years, 9 months ago (2013-02-27 17:04:29 UTC) #14
gary.poster
On 2013/02/27 17:04:29, jeff.pihach wrote: > Thanks for the reviews! Please see the attached comments ...
7 years, 9 months ago (2013-02-27 17:20:31 UTC) #15
bcsaller
LGTM I think this is in shape to land so we can move forward with ...
7 years, 9 months ago (2013-02-27 17:22:14 UTC) #16
jeff.pihach
Hey sorry guys I forgot to 'save' my comments on the test page - please ...
7 years, 9 months ago (2013-02-27 17:23:24 UTC) #17
jeff.pihach
Please take a look.
7 years, 9 months ago (2013-02-27 17:48:44 UTC) #18
gary.poster
LGTM still--looks even better. :-) Thanks Gary https://codereview.appspot.com/7381055/diff/23001/app/assets/javascripts/app-subapp-extension.js File app/assets/javascripts/app-subapp-extension.js (right): https://codereview.appspot.com/7381055/diff/23001/app/assets/javascripts/app-subapp-extension.js#newcode50 app/assets/javascripts/app-subapp-extension.js:50: @param {string} ...
7 years, 9 months ago (2013-02-27 18:23:23 UTC) #19
jeff.pihach
*** Submitted: Created app subapp extension R=bcsaller, gary.poster CC= https://codereview.appspot.com/7381055
7 years, 9 months ago (2013-02-27 20:35:09 UTC) #20
jeff.pihach
7 years, 9 months ago (2013-02-27 21:21:07 UTC) #21
Please take a look.
Sign in to reply to this message.

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