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

Issue 8626043: juju: remove dependency on api/params

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 7 months ago by rog
Modified:
9 years, 7 months ago
Reviewers:
dimitern, mp+158137, fwereade
Visibility:
Public.

Description

juju: remove dependency on api/params The juju package should really not depend on api/params if possible. We move two functions into State and delete the now-redundant statecmd shim functions. We also fix bug 1167344 in passing. The only logic change in this branch is to remove "no options to set" check from Service.Set. IMHO setting no options should be acceptable. https://code.launchpad.net/~rogpeppe/juju-core/278-juju-no-params/+merge/158137 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : juju: remove dependency on api/params #

Patch Set 3 : juju: remove dependency on api/params #

Total comments: 15

Patch Set 4 : juju: remove dependency on api/params #

Patch Set 5 : juju: remove dependency on api/params #

Unified diffs Side-by-side diffs Delta from patch set Stats (+362 lines, -528 lines) Patch
A [revision details] View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/resolved.go View 2 chunks +4 lines, -6 lines 0 comments Download
M cmd/juju/resolved_test.go View 3 chunks +8 lines, -7 lines 0 comments Download
M cmd/juju/set.go View 1 2 3 2 chunks +6 lines, -9 lines 0 comments Download
M juju/conn.go View 1 2 3 3 chunks +6 lines, -109 lines 0 comments Download
M juju/conn_test.go View 2 chunks +0 lines, -33 lines 0 comments Download
M state/api/params/params.go View 2 chunks +0 lines, -11 lines 0 comments Download
M state/api/params/params_test.go View 1 chunk +1 line, -2 lines 0 comments Download
M state/apiserver/api_test.go View 1 chunk +1 line, -1 line 0 comments Download
M state/apiserver/apiserver.go View 1 2 3 2 chunks +15 lines, -3 lines 0 comments Download
M state/megawatcher.go View 1 chunk +0 lines, -1 line 0 comments Download
M state/megawatcher_internal_test.go View 2 chunks +0 lines, -3 lines 0 comments Download
M state/service.go View 1 2 3 2 chunks +72 lines, -0 lines 0 comments Download
M state/service_test.go View 1 2 3 1 chunk +124 lines, -0 lines 0 comments Download
D state/statecmd/config.go View 1 chunk +0 lines, -25 lines 0 comments Download
M state/statecmd/config_test.go View 2 chunks +0 lines, -169 lines 0 comments Download
D state/statecmd/resolved.go View 1 chunk +0 lines, -34 lines 0 comments Download
D state/statecmd/resolved_test.go View 1 chunk +0 lines, -44 lines 0 comments Download
M state/unit.go View 5 chunks +37 lines, -7 lines 0 comments Download
M state/unit_test.go View 2 chunks +48 lines, -25 lines 0 comments Download
M worker/uniter/filter.go View 7 chunks +8 lines, -9 lines 0 comments Download
M worker/uniter/filter_test.go View 4 chunks +6 lines, -6 lines 0 comments Download
M worker/uniter/modes.go View 1 chunk +2 lines, -2 lines 0 comments Download
M worker/uniter/uniter_test.go View 22 chunks +22 lines, -22 lines 0 comments Download

Messages

Total messages: 7
rog
Please take a look.
9 years, 7 months ago (2013-04-10 15:07:44 UTC) #1
rog
Please take a look.
9 years, 7 months ago (2013-04-10 15:08:19 UTC) #2
fwereade
LGTM with the following resolved. Ping me if you disagree :) https://codereview.appspot.com/8626043/diff/5001/juju/conn.go File juju/conn.go (right): ...
9 years, 7 months ago (2013-04-10 16:24:24 UTC) #3
dimitern
LGTM, nice to see temporary and redundant code going away. https://codereview.appspot.com/8626043/diff/5001/state/service_test.go File state/service_test.go (right): https://codereview.appspot.com/8626043/diff/5001/state/service_test.go#newcode201 ...
9 years, 7 months ago (2013-04-10 16:28:31 UTC) #4
rog
Please take a look. https://codereview.appspot.com/8626043/diff/5001/juju/conn.go File juju/conn.go (right): https://codereview.appspot.com/8626043/diff/5001/juju/conn.go#newcode202 juju/conn.go:202: // subsequent operations? On 2013/04/10 ...
9 years, 7 months ago (2013-04-10 17:40:35 UTC) #5
rog
i believe we discussed all the issues except the method vs. function issue. since a ...
9 years, 7 months ago (2013-04-10 17:46:13 UTC) #6
rog
9 years, 7 months ago (2013-04-10 17:49:22 UTC) #7
*** Submitted:

juju: remove dependency on api/params

The juju package should really not depend on api/params
if possible. We move two functions into State and
delete the now-redundant statecmd shim functions.

We also fix bug 1167344 in passing.

The only logic change in this branch is to remove
"no options to set" check from Service.Set.
IMHO setting no options should be acceptable.

R=fwereade, dimitern
CC=
https://codereview.appspot.com/8626043
Sign in to reply to this message.

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