Code review comment for lp:~themue/pyjuju/go-state-add-relation

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

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 is internal. :-)

https://codereview.appspot.com/6223055/diff/1/state/relation.go#newcode73
state/relation.go:73: // Key returns the internal key of a relation.
Ditto.

https://codereview.appspot.com/6223055/diff/1/state/relation.go#newcode78
state/relation.go:78: // ServiceKey returns the service key of a
relation.
Ditto.

https://codereview.appspot.com/6223055/diff/1/state/state.go
File state/state.go (right):

https://codereview.appspot.com/6223055/diff/1/state/state.go#newcode239
state/state.go:239: if clientEp.RelationRole != RoleClient {
As a general and vague comment, visually these functions do not look
nice. Maybe there's no way to avoid the complexity, but please look for
opportunities to separate logical blocks visually, and perhaps using
functions is adequate.

I'll hold-off on reviewing this until we sort out the pre-req, since
there are necessary changes there that will certainly affect this.

https://codereview.appspot.com/6223055/

« Back to merge proposal