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

Issue 7945044: statecmd: don't use juju.Conn unnecessarily

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

Description

statecmd: don't use juju.Conn unnecessarily There's an associated cost when the API uses juju.Conn, so we want to avoid it when reasonable. This change moves some operations from juju into state because they don't require juju.Conn's associated Environ. https://code.launchpad.net/~rogpeppe/juju-core/251-statecmd-remove-unnecessary-conn/+merge/154744 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : statecmd: don't use juju.Conn unnecessarily #

Total comments: 1

Patch Set 3 : statecmd: don't use juju.Conn unnecessarily #

Unified diffs Side-by-side diffs Delta from patch set Stats (+169 lines, -190 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/destroymachine.go View 1 chunk +1 line, -1 line 0 comments Download
M juju/conn.go View 1 2 3 chunks +2 lines, -57 lines 0 comments Download
M juju/conn_test.go View 1 2 1 chunk +0 lines, -119 lines 0 comments Download
M state/machine_test.go View 1 chunk +35 lines, -0 lines 0 comments Download
M state/state.go View 1 chunk +57 lines, -0 lines 0 comments Download
M state/statecmd/destroyunit.go View 1 chunk +2 lines, -7 lines 0 comments Download
M state/statecmd/setconstraints.go View 1 chunk +1 line, -6 lines 0 comments Download
M state/unit_test.go View 1 2 1 chunk +69 lines, -0 lines 0 comments Download

Messages

Total messages: 4
rog
Please take a look.
12 years ago (2013-03-21 16:03:08 UTC) #1
fwereade
LGTM https://codereview.appspot.com/7945044/diff/2001/state/state.go File state/state.go (right): https://codereview.appspot.com/7945044/diff/2001/state/state.go#newcode757 state/state.go:757: // TODO(rog) make this a transaction? Probably a ...
12 years ago (2013-03-22 00:37:23 UTC) #2
dimitern
LGTM
12 years ago (2013-03-22 13:21:31 UTC) #3
rog
12 years ago (2013-03-22 17:10:50 UTC) #4
*** Submitted:

statecmd: don't use juju.Conn unnecessarily

There's an associated cost when the API uses juju.Conn,
so we want to avoid it when reasonable.
This change moves some operations from juju into
state because they don't require juju.Conn's associated
Environ.

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

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