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

Issue 5727045: Continuation of the unit state implementation. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years ago by TheMue
Modified:
12 years ago
Reviewers:
mp+95684
Visibility:
Public.

Description

The unit state isn't yet finalized. This branch contains more methods regarding upgrade, resolved and open ports. Additionally the replacement of ZooKeeper connections inside the entities by references to the state has been continued. https://code.launchpad.net/~themue/juju/go-state-continued-unit/+merge/95684 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 41

Patch Set 2 : Continuation of the unit state implementation. #

Total comments: 10

Patch Set 3 : Continuation of the unit state implementation. #

Total comments: 32

Patch Set 4 : Continuation of the unit state implementation. #

Total comments: 8

Patch Set 5 : Continuation of the unit state implementation. #

Patch Set 6 : Continuation of the unit state implementation. #

Patch Set 7 : Continuation of the unit state implementation. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+345 lines, -19 lines) Patch
state/machine.go View 1 2 3 4 5 6 1 chunk +1 line, -2 lines 0 comments Download
state/service.go View 1 2 3 4 5 6 3 chunks +3 lines, -3 lines 0 comments Download
state/state.go View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
state/state_test.go View 1 2 3 4 5 6 1 chunk +137 lines, -0 lines 0 comments Download
state/unit.go View 1 2 3 4 5 6 14 chunks +203 lines, -13 lines 0 comments Download

Messages

Total messages: 18
TheMue
Please take a look.
12 years ago (2012-03-02 22:30:55 UTC) #1
fwereade
https://codereview.appspot.com/5727045/diff/1/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/1/state/unit.go#newcode218 state/unit.go:218: return retryTopologyChange(u.st.zk, unassignUnit) Unrelated to this merge; but wouldn't ...
12 years ago (2012-03-09 11:48:02 UTC) #2
rog
https://codereview.appspot.com/5727045/diff/1/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/1/state/unit.go#newcode18 state/unit.go:18: ResolvedRetryHooks = 1000 i think these should get their ...
12 years ago (2012-03-09 15:05:20 UTC) #3
TheMue
Most comments are fine and will rush in with the next propose. https://codereview.appspot.com/5727045/diff/1/state/unit.go File state/unit.go ...
12 years ago (2012-03-09 15:26:43 UTC) #4
fwereade
https://codereview.appspot.com/5727045/diff/1/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/1/state/unit.go#newcode264 state/unit.go:264: } On 2012/03/09 15:26:43, TheMue wrote: > On 2012/03/09 ...
12 years ago (2012-03-09 15:38:49 UTC) #5
rog
https://codereview.appspot.com/5727045/diff/1/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/1/state/unit.go#newcode342 state/unit.go:342: changedOpen := make([]PortProto, 0) On 2012/03/09 15:26:43, TheMue wrote: ...
12 years ago (2012-03-09 16:20:28 UTC) #6
TheMue
Please take a look.
12 years ago (2012-03-09 18:37:02 UTC) #7
TheMue
https://codereview.appspot.com/5727045/diff/1/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/1/state/unit.go#newcode18 state/unit.go:18: ResolvedRetryHooks = 1000 On 2012/03/09 15:05:20, rog wrote: > ...
12 years ago (2012-03-09 18:43:45 UTC) #8
fwereade
Basically LGTM, but I'd like the chance to talk about naming a little; grab me ...
12 years ago (2012-03-10 17:35:32 UTC) #9
TheMue
Please take a look.
12 years ago (2012-03-12 14:38:12 UTC) #10
TheMue
PTAL https://codereview.appspot.com/5727045/diff/7001/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/7001/state/unit.go#newcode18 state/unit.go:18: // variable inside On 2012/03/10 17:35:32, fwereade wrote: ...
12 years ago (2012-03-12 14:42:33 UTC) #11
niemeyer
Looks pretty good. Some minor details only: https://codereview.appspot.com/5727045/diff/11001/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/11001/state/unit.go#newcode19 state/unit.go:19: type ErrorResolution ...
12 years ago (2012-03-12 15:38:38 UTC) #12
TheMue
Please take a look.
12 years ago (2012-03-12 17:36:55 UTC) #13
niemeyer
LGTM. Just a few extra trivials before submitting please: https://codereview.appspot.com/5727045/diff/9003/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/9003/state/unit.go#newcode255 state/unit.go:255: ...
12 years ago (2012-03-12 18:13:14 UTC) #14
TheMue
https://codereview.appspot.com/5727045/diff/11001/state/unit.go File state/unit.go (right): https://codereview.appspot.com/5727045/diff/11001/state/unit.go#newcode19 state/unit.go:19: type ErrorResolution int On 2012/03/12 15:38:39, niemeyer wrote: > ...
12 years ago (2012-03-12 18:33:01 UTC) #15
TheMue
Please take a look.
12 years ago (2012-03-12 18:34:49 UTC) #16
TheMue
Please take a look.
12 years ago (2012-03-12 19:02:01 UTC) #17
TheMue
12 years ago (2012-03-12 19:07:18 UTC) #18
*** Submitted:

Continuation of the unit state implementation.

The unit state isn't yet finalized. This branch contains
more methods regarding upgrade, resolved and open ports.
Additionally the replacement of ZooKeeper connections 
inside the entities by references to the state has been
continued.

R=fwereade, rog, niemeyer
CC=
https://codereview.appspot.com/5727045
Sign in to reply to this message.

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