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

Issue 6501114: mstate/presence: bring it in line with mstate/watcher

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 3 months ago by niemeyer
Modified:
10 years, 3 months ago
Reviewers:
mp+123627, fwereade
Visibility:
Public.

Description

mstate/presence: bring it in line with mstate/watcher This mirrors into mstate/presence the improvements made into the mstate/watcher package, including several points made in reviews. It also addresses a bug in Alive (it could return false improperly without errors). https://code.launchpad.net/~niemeyer/juju-core/presence-polishing/+merge/123627 Requires: https://code.launchpad.net/~niemeyer/juju-core/mstate-machine-watcher/+merge/123614 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 9

Patch Set 2 : mstate/presence: bring it in line with mstate/watcher #

Patch Set 3 : mstate/presence: bring it in line with mstate/watcher #

Unified diffs Side-by-side diffs Delta from patch set Stats (+321 lines, -209 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M mstate/machine.go View 1 1 chunk +19 lines, -25 lines 0 comments Download
M mstate/machine_test.go View 2 chunks +13 lines, -10 lines 0 comments Download
M mstate/open.go View 2 chunks +3 lines, -3 lines 0 comments Download
M mstate/presence/presence.go View 9 chunks +88 lines, -74 lines 0 comments Download
M mstate/presence/presence_test.go View 10 chunks +137 lines, -37 lines 0 comments Download
M mstate/state.go View 1 2 chunks +19 lines, -19 lines 0 comments Download
M mstate/unit.go View 1 2 chunks +23 lines, -26 lines 0 comments Download
M mstate/unit_test.go View 3 chunks +15 lines, -10 lines 0 comments Download
M mstate/watcher/watcher_test.go View 3 chunks +2 lines, -5 lines 0 comments Download

Messages

Total messages: 5
niemeyer
Please take a look.
10 years, 3 months ago (2012-09-10 19:29:37 UTC) #1
fwereade
LGTM, only very vague suggestions :). https://codereview.appspot.com/6501114/diff/1/mstate/machine.go File mstate/machine.go (left): https://codereview.appspot.com/6501114/diff/1/mstate/machine.go#oldcode88 mstate/machine.go:88: func (m *Machine) ...
10 years, 3 months ago (2012-09-11 12:09:31 UTC) #2
niemeyer
Please take a look. https://codereview.appspot.com/6501114/diff/1/mstate/machine.go File mstate/machine.go (left): https://codereview.appspot.com/6501114/diff/1/mstate/machine.go#oldcode88 mstate/machine.go:88: func (m *Machine) WaitAgentAlive(timeout time.Duration) ...
10 years, 3 months ago (2012-09-11 13:27:15 UTC) #3
fwereade
LGTM https://codereview.appspot.com/6501114/diff/1/mstate/machine.go File mstate/machine.go (right): https://codereview.appspot.com/6501114/diff/1/mstate/machine.go#newcode114 mstate/machine.go:114: return fmt.Errorf("waiting for agent of machine %v: watcher ...
10 years, 3 months ago (2012-09-11 13:36:45 UTC) #4
niemeyer
10 years, 3 months ago (2012-09-11 13:45:08 UTC) #5
*** Submitted:

mstate/presence: bring it in line with mstate/watcher

This mirrors into mstate/presence the improvements made into
the mstate/watcher package, including several points made in
reviews.

It also addresses a bug in Alive (it could return false
improperly without errors).

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

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