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

Issue 6223055: state: Added two methods to add relations to State. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 1 month ago by TheMue
Modified:
9 years ago
Reviewers:
mp+106652
Visibility:
Public.

Description

state: Added two methods to add relations to State. AddClientServerReleation() gets a client and a server endpoint, AddPeerRelation() a peer endpoint. They return the relation and both service relations for client/server or the one for peer. The seperation into two methods with well defined arguments make it easier to ensure a valid relation creation. https://code.launchpad.net/~themue/juju/go-state-add-relation/+merge/106652 Requires: https://code.launchpad.net/~themue/juju/go-state-topology-relation-endpoints/+merge/105156 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 7

Patch Set 2 : Added two methods to add relations to State. #

Patch Set 3 : Added two methods to add relations to State. #

Total comments: 52

Patch Set 4 : state: Added two methods to add relations to State. #

Patch Set 5 : state: Added two methods to add relations to State. #

Patch Set 6 : state: Added two methods to add relations to State. #

Total comments: 5

Patch Set 7 : state: Added two methods to add relations to State. #

Patch Set 8 : state: Added two methods to add relations to State. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -3 lines) Patch
A [revision details] View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M state/relation.go View 1 2 3 1 chunk +33 lines, -0 lines 0 comments Download
M state/state.go View 1 2 3 4 5 6 7 1 chunk +117 lines, -0 lines 0 comments Download
M state/state_test.go View 1 2 3 4 5 6 7 2 chunks +100 lines, -3 lines 0 comments Download

Messages

Total messages: 12
TheMue
Please take a look.
9 years, 1 month ago (2012-05-21 16:21:47 UTC) #1
niemeyer
https://codereview.appspot.com/6223055/diff/1/state/relation.go File state/relation.go (right): https://codereview.appspot.com/6223055/diff/1/state/relation.go#newcode60 state/relation.go:60: func (r *Relation) Key() string { The internal key ...
9 years, 1 month ago (2012-05-21 20:07:47 UTC) #2
TheMue
Please take a look. https://codereview.appspot.com/6223055/diff/1/state/relation.go File state/relation.go (right): https://codereview.appspot.com/6223055/diff/1/state/relation.go#newcode60 state/relation.go:60: func (r *Relation) Key() string ...
9 years ago (2012-05-29 15:55:13 UTC) #3
TheMue
Please take a look.
9 years ago (2012-05-30 08:20:25 UTC) #4
niemeyer
https://codereview.appspot.com/6223055/diff/9001/state/relation.go File state/relation.go (right): https://codereview.appspot.com/6223055/diff/9001/state/relation.go#newcode50 state/relation.go:50: // Relation represents a connection between one or more ...
9 years ago (2012-05-30 22:35:42 UTC) #5
TheMue
Please take a look. https://codereview.appspot.com/6223055/diff/9001/state/relation.go File state/relation.go (right): https://codereview.appspot.com/6223055/diff/9001/state/relation.go#newcode50 state/relation.go:50: // Relation represents a connection ...
9 years ago (2012-05-31 16:23:59 UTC) #6
niemeyer
The code diff has a lot of unrelated code. Please let me know once the ...
9 years ago (2012-06-01 11:58:12 UTC) #7
TheMue
Please take a look.
9 years ago (2012-06-01 12:05:05 UTC) #8
TheMue
Please take a look.
9 years ago (2012-06-01 12:06:36 UTC) #9
niemeyer
LGTM, but see the comments please: https://codereview.appspot.com/6223055/diff/9001/state/state.go File state/state.go (right): https://codereview.appspot.com/6223055/diff/9001/state/state.go#newcode276 state/state.go:276: if scope == ...
9 years ago (2012-06-01 12:21:52 UTC) #10
TheMue
Please take a look. https://codereview.appspot.com/6223055/diff/10005/state/state.go File state/state.go (right): https://codereview.appspot.com/6223055/diff/10005/state/state.go#newcode263 state/state.go:263: // container occurs in ServiceRelation.AddUnit(). ...
9 years ago (2012-06-01 12:41:39 UTC) #11
TheMue
9 years ago (2012-06-04 14:51:07 UTC) #12
*** Submitted:

state: Added two methods to add relations to State.

AddClientServerReleation() gets a client and a server endpoint,
AddPeerRelation() a peer endpoint. They return the relation and
both service relations for client/server or the one for peer.
The seperation into two methods with well defined arguments make
it easier to ensure a valid relation creation.

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

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