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

Issue 7948043: state: add annotations to StateWatcher

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

Description

state: add annotations to StateWatcher https://code.launchpad.net/~rogpeppe/juju-core/250-allwatcher-annotations/+merge/154752 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : state: add annotations to StateWatcher #

Total comments: 3

Patch Set 3 : state: add annotations to StateWatcher #

Unified diffs Side-by-side diffs Delta from patch set Stats (+57 lines, -11 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M state/api/params/params.go View 3 chunks +18 lines, -3 lines 0 comments Download
M state/api/params/params_test.go View 1 2 5 chunks +13 lines, -6 lines 0 comments Download
M state/megawatcher.go View 1 chunk +3 lines, -0 lines 0 comments Download
M state/megawatcher_internal_test.go View 1 2 4 chunks +21 lines, -2 lines 0 comments Download

Messages

Total messages: 4
rog
Please take a look.
11 years, 1 month ago (2013-03-21 17:02:32 UTC) #1
fwereade
LGTM. https://codereview.appspot.com/7948043/diff/2001/state/api/params/params.go File state/api/params/params.go (right): https://codereview.appspot.com/7948043/diff/2001/state/api/params/params.go#newcode285 state/api/params/params.go:285: // for the API by specifying the json ...
11 years, 1 month ago (2013-03-22 00:42:36 UTC) #2
dimitern
LGTM
11 years, 1 month ago (2013-03-22 13:19:07 UTC) #3
rog
11 years, 1 month ago (2013-03-22 16:53:54 UTC) #4
*** Submitted:

state: add annotations to StateWatcher

R=fwereade, dimitern
CC=
https://codereview.appspot.com/7948043

https://codereview.appspot.com/7948043/diff/2001/state/api/params/params_test.go
File state/api/params/params_test.go (right):

https://codereview.appspot.com/7948043/diff/2001/state/api/params/params_test...
state/api/params/params_test.go:67: "arble": "",
On 2013/03/22 00:42:36, fwereade wrote:
> Is this sane? I think that any set-to-"" is interpreted as a delete, so we
> shouldn't be seeing maps like this at all, should we?
> 
> I know it's just test data though.

Done.
Sign in to reply to this message.

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