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

Issue 7399049: state: add Machine.Series

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by fwereade
Modified:
11 years, 1 month ago
Reviewers:
dimitern, mp+150001, TheMue
Visibility:
Public.

Description

state: add Machine.Series as a very first step towards resolving lp:1131608 https://code.launchpad.net/~fwereade/juju-core/state-machine-series/+merge/150001 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : state: add Machine.Series #

Unified diffs Side-by-side diffs Delta from patch set Stats (+169 lines, -141 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/destroymachine_test.go View 1 chunk +1 line, -1 line 0 comments Download
M cmd/juju/ssh_test.go View 1 chunk +1 line, -1 line 0 comments Download
M cmd/juju/status_test.go View 2 chunks +2 lines, -2 lines 0 comments Download
M cmd/jujud/bootstrap.go View 2 chunks +4 lines, -1 line 0 comments Download
M cmd/jujud/machine_test.go View 1 chunk +1 line, -1 line 0 comments Download
M juju/conn_test.go View 1 chunk +5 lines, -5 lines 0 comments Download
M state/api/api_test.go View 10 chunks +17 lines, -17 lines 0 comments Download
M state/assign_test.go View 23 chunks +26 lines, -26 lines 0 comments Download
M state/life_test.go View 1 chunk +1 line, -1 line 0 comments Download
M state/machine.go View 1 2 chunks +6 lines, -0 lines 0 comments Download
M state/machine_test.go View 7 chunks +9 lines, -9 lines 0 comments Download
M state/service_test.go View 1 chunk +1 line, -1 line 0 comments Download
M state/state.go View 1 3 chunks +18 lines, -13 lines 0 comments Download
M state/state_test.go View 10 chunks +54 lines, -42 lines 0 comments Download
M state/tools_test.go View 1 chunk +1 line, -1 line 0 comments Download
M state/unit_test.go View 1 chunk +1 line, -1 line 0 comments Download
M worker/deployer/deployer_test.go View 2 chunks +2 lines, -2 lines 0 comments Download
M worker/firewaller/firewaller_test.go View 3 chunks +3 lines, -3 lines 0 comments Download
M worker/machiner/machiner_test.go View 2 chunks +2 lines, -2 lines 0 comments Download
M worker/provisioner/provisioner_test.go View 11 chunks +12 lines, -12 lines 0 comments Download

Messages

Total messages: 5
fwereade
Please take a look.
11 years, 1 month ago (2013-02-22 10:04:07 UTC) #1
TheMue
LGTM, only minor comments regarding comments. https://codereview.appspot.com/7399049/diff/1/state/machine.go File state/machine.go (right): https://codereview.appspot.com/7399049/diff/1/state/machine.go#newcode69 state/machine.go:69: func (m *Machine) ...
11 years, 1 month ago (2013-02-22 10:18:17 UTC) #2
dimitern
LGTM. I have a question though - how "series" is different from version.Current.Series?
11 years, 1 month ago (2013-02-22 10:31:18 UTC) #3
fwereade
On 2013/02/22 10:31:18, dimitern wrote: > LGTM. > > I have a question though - ...
11 years, 1 month ago (2013-02-22 10:39:11 UTC) #4
fwereade
11 years, 1 month ago (2013-02-22 10:46:48 UTC) #5
*** Submitted:

state: add Machine.Series

as a very first step towards resolving lp:1131608

R=TheMue, dimitern
CC=
https://codereview.appspot.com/7399049

https://codereview.appspot.com/7399049/diff/1/state/machine.go
File state/machine.go (right):

https://codereview.appspot.com/7399049/diff/1/state/machine.go#newcode69
state/machine.go:69: func (m *Machine) Series() string {
On 2013/02/22 10:18:17, TheMue wrote:
> Missing comment (even if it's obvious ;) ).

Thanks, done.

https://codereview.appspot.com/7399049/diff/1/state/state.go
File state/state.go (right):

https://codereview.appspot.com/7399049/diff/1/state/state.go#newcode161
state/state.go:161: // AddMachine adds a new machine configured to run the
supplied jobs.
On 2013/02/22 10:18:17, TheMue wrote:
> Add the role of "series" here like it's done for "jobs".

Done.
Sign in to reply to this message.

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