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

Issue 24790044: fix lp:1089291

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 4 months ago by fwereade
Modified:
10 years, 4 months ago
Reviewers:
mp+194764, jameinel, wallyworld
Visibility:
Public.

Description

fix lp:1089291 ...which involved a lot more churn than I wanted, mainly because those DestroyWhatevers methods really shouldn't ever have been on state in the first place, and moving them turned out to be a ridiculous hassle. Despite the mess, this branch really just involves: 1) moving multi-Destroy methods off state and onto the api 2) fixing/moving/renaming test things 3) hooking up a --force flag to destroy-machine 4) oh, and making the relevant api.Client methods a teeny tiny bit consistent. https://code.launchpad.net/~fwereade/juju-core/api-force-destroy-machines/+merge/194764 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 14

Patch Set 2 : fix lp:1089291 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+371 lines, -287 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/destroymachine.go View 1 3 chunks +18 lines, -1 line 0 comments Download
M cmd/juju/destroymachine_test.go View 1 5 chunks +51 lines, -14 lines 0 comments Download
M cmd/juju/destroyunit.go View 1 chunk +1 line, -1 line 0 comments Download
M cmd/jujud/machine_test.go View 1 chunk +1 line, -1 line 0 comments Download
M state/api/client.go View 2 chunks +7 lines, -1 line 0 comments Download
M state/api/params/params.go View 1 chunk +1 line, -0 lines 0 comments Download
M state/apiserver/client/client.go View 4 chunks +52 lines, -2 lines 0 comments Download
M state/apiserver/client/client_test.go View 1 chunk +136 lines, -31 lines 0 comments Download
M state/apiserver/client/perm_test.go View 1 chunk +1 line, -1 line 0 comments Download
M state/cleanup_test.go View 1 6 chunks +26 lines, -8 lines 0 comments Download
M state/life.go View 1 chunk +8 lines, -1 line 0 comments Download
M state/life_test.go View 5 chunks +5 lines, -15 lines 0 comments Download
M state/machine.go View 2 chunks +20 lines, -3 lines 0 comments Download
M state/machine_test.go View 5 chunks +13 lines, -44 lines 0 comments Download
M state/service_test.go View 2 chunks +3 lines, -3 lines 0 comments Download
M state/state.go View 1 chunk +0 lines, -67 lines 0 comments Download
M state/unit_test.go View 9 chunks +26 lines, -94 lines 0 comments Download

Messages

Total messages: 7
fwereade
Please take a look.
10 years, 4 months ago (2013-11-11 23:10:26 UTC) #1
wallyworld
Some nice refactoring. It does make me a little sad that we are putting service ...
10 years, 4 months ago (2013-11-12 00:43:42 UTC) #2
jameinel
just some of my thoughts, not a full review. https://codereview.appspot.com/24790044/diff/1/state/api/client.go File state/api/client.go (right): https://codereview.appspot.com/24790044/diff/1/state/api/client.go#newcode154 state/api/client.go:154: ...
10 years, 4 months ago (2013-11-12 02:58:37 UTC) #3
fwereade
Machine lifecycle is out of scope; SOA concepts demand more discussion in a separate forum; ...
10 years, 4 months ago (2013-11-12 10:08:11 UTC) #4
fwereade
Please take a look.
10 years, 4 months ago (2013-11-12 10:24:59 UTC) #5
wallyworld
LGTM
10 years, 4 months ago (2013-11-12 10:45:39 UTC) #6
jameinel
10 years, 4 months ago (2013-11-12 10:52:17 UTC) #7
LGTM as well
Sign in to reply to this message.

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