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

Issue 8532043: Augment ServiceGetResults

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by bac
Modified:
10 years, 11 months ago
Reviewers:
teknico, mp+157751, rog
Visibility:
Public.

Description

Augment ServiceGetResults The GUI needs constraints from the API call. Also change s/Settings/Config to match the Python version of the API call to avoid needless mapping changes on the client side. https://code.launchpad.net/~bac/juju-core/1164593/+merge/157751 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : Augment ServiceGetResults #

Unified diffs Side-by-side diffs Delta from patch set Stats (+30 lines, -16 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/get.go View 1 chunk +1 line, -1 line 0 comments Download
M state/api/params/params.go View 1 1 chunk +4 lines, -3 lines 0 comments Download
M state/apiserver/api_test.go View 1 1 chunk +3 lines, -3 lines 0 comments Download
M state/statecmd/config_test.go View 1 4 chunks +9 lines, -4 lines 0 comments Download
M state/statecmd/get.go View 2 chunks +11 lines, -5 lines 0 comments Download

Messages

Total messages: 5
bac
Please take a look.
10 years, 11 months ago (2013-04-08 20:57:53 UTC) #1
teknico
LGTM https://codereview.appspot.com/8532043/diff/1/state/statecmd/get.go File state/statecmd/get.go (right): https://codereview.appspot.com/8532043/diff/1/state/statecmd/get.go#newcode23 state/statecmd/get.go:23: svcCfg, err := svc.Config() Nice. https://codereview.appspot.com/8532043/diff/1/state/statecmd/get.go#newcode31 state/statecmd/get.go:31: charmCfg ...
10 years, 11 months ago (2013-04-09 13:16:50 UTC) #2
rog
LGTM with one remark for future consideration. nice. https://codereview.appspot.com/8532043/diff/1/state/api/params/params.go File state/api/params/params.go (right): https://codereview.appspot.com/8532043/diff/1/state/api/params/params.go#newcode67 state/api/params/params.go:67: Charm ...
10 years, 11 months ago (2013-04-09 14:09:39 UTC) #3
bac
*** Submitted: Augment ServiceGetResults The GUI needs constraints from the API call. Also change s/Settings/Config ...
10 years, 11 months ago (2013-04-09 14:41:43 UTC) #4
bac
10 years, 11 months ago (2013-04-09 14:44:23 UTC) #5
Thanks for the reviews.  Rog I did inline as you suggested.
Sign in to reply to this message.

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