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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by TheMue
Modified:
10 years, 3 months 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.
10 years, 3 months 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 ...
10 years, 3 months 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 ...
10 years, 3 months 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 ...
10 years, 3 months 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 ...
10 years, 3 months 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: ...
10 years, 3 months ago (2012-03-09 16:20:28 UTC) #6
TheMue
Please take a look.
10 years, 3 months 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: > ...
10 years, 3 months 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 ...
10 years, 3 months ago (2012-03-10 17:35:32 UTC) #9
TheMue
Please take a look.
10 years, 3 months 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: ...
10 years, 3 months 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 ...
10 years, 3 months ago (2012-03-12 15:38:38 UTC) #12
TheMue
Please take a look.
10 years, 3 months 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: ...
10 years, 3 months 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: > ...
10 years, 3 months ago (2012-03-12 18:33:01 UTC) #15
TheMue
Please take a look.
10 years, 3 months ago (2012-03-12 18:34:49 UTC) #16
TheMue
Please take a look.
10 years, 3 months ago (2012-03-12 19:02:01 UTC) #17
TheMue
10 years, 3 months 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