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

Issue 8487044: state: separate EntityInfo from docs

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by rog
Modified:
10 years, 11 months ago
Reviewers:
dimitern, frankban, mp+157705, TheMue
Visibility:
Public.

Description

state: separate EntityInfo from docs This is the next stage along the road to being able to merge separate documents for consumption by the AllWatcher. https://code.launchpad.net/~rogpeppe/juju-core/273-megawatcher-merge-attrs/+merge/157705 Requires: https://code.launchpad.net/~rogpeppe/juju-core/271-megawatcher-factor-out/+merge/157656 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : state: separate EntityInfo from docs #

Patch Set 3 : state: separate EntityInfo from docs #

Patch Set 4 : state: separate EntityInfo from docs #

Patch Set 5 : state: separate EntityInfo from docs #

Patch Set 6 : state: separate EntityInfo from docs #

Total comments: 6

Patch Set 7 : state: separate EntityInfo from docs #

Unified diffs Side-by-side diffs Delta from patch set Stats (+718 lines, -386 lines) Patch
A [revision details] View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M state/api/params/params.go View 8 chunks +59 lines, -34 lines 0 comments Download
M state/api/params/params_test.go View 1 chunk +4 lines, -10 lines 0 comments Download
M state/megawatcher.go View 1 2 3 4 5 6 3 chunks +186 lines, -76 lines 0 comments Download
M state/megawatcher_internal_test.go View 1 2 3 4 5 8 chunks +341 lines, -134 lines 0 comments Download
M state/multiwatcher/multiwatcher.go View 1 2 3 4 5 6 8 chunks +11 lines, -18 lines 0 comments Download
M state/multiwatcher/multiwatcher_internal_test.go View 1 2 3 4 5 28 chunks +97 lines, -114 lines 0 comments Download
M state/state.go View 1 2 3 4 5 1 chunk +18 lines, -0 lines 0 comments Download

Messages

Total messages: 8
rog
Please take a look.
10 years, 11 months ago (2013-04-08 17:27:37 UTC) #1
dimitern
Even nicer! LGTM
10 years, 11 months ago (2013-04-08 17:37:10 UTC) #2
rog
Please take a look.
10 years, 11 months ago (2013-04-08 19:39:23 UTC) #3
rog
Please take a look.
10 years, 11 months ago (2013-04-08 22:37:07 UTC) #4
rog
Please take a look.
10 years, 11 months ago (2013-04-09 08:07:47 UTC) #5
TheMue
LGTM. I btw lke the strategy pattern used for the store backing. Helps in tests. ...
10 years, 11 months ago (2013-04-09 09:24:12 UTC) #6
frankban
LGTM, thank you Roger, this separation adds more flexibility to the allwatcher API. See just ...
10 years, 11 months ago (2013-04-09 09:35:49 UTC) #7
rog
10 years, 11 months ago (2013-04-09 10:12:48 UTC) #8
*** Submitted:

state: separate EntityInfo from docs

This is the next stage along the road to
being able to merge separate documents for
consumption by the AllWatcher.

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

https://codereview.appspot.com/8487044/diff/15001/state/megawatcher.go
File state/megawatcher.go (right):

https://codereview.appspot.com/8487044/diff/15001/state/megawatcher.go#newcod...
state/megawatcher.go:161: updated(st *State, store *multiwatcher.Store) error
On 2013/04/09 09:35:50, frankban wrote:
> Just curiosity: why are the updated and removed methods  required to return an
> error when all the implementations above always return nil?

subsequent branches will have implementations that need to return an error.
that's also why we're passing in the State.

https://codereview.appspot.com/8487044/diff/15001/state/megawatcher.go#newcod...
state/megawatcher.go:163: // updated is called when the document has changed.
On 2013/04/09 09:35:50, frankban wrote:
> This comment should describe the removed method I guess.

Done.
Sign in to reply to this message.

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