Code review comment for lp:~dimitern/juju-core/103-relation-entity-tags

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM with some extra tests.

https://codereview.appspot.com/12748044/diff/1/names/relation.go
File names/relation.go (right):

https://codereview.appspot.com/12748044/diff/1/names/relation.go#newcode18
names/relation.go:18: func RelationTag(relationId string) string {
if !IsRelation(relationId) {
    panic(fmt.Sprintf("%q is not a valid relation id", relationId))
}

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

https://codereview.appspot.com/12748044/diff/1/state/relation.go#newcode66
state/relation.go:66: func (r *Relation) Tag() string {
This should have a test.

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

https://codereview.appspot.com/12748044/diff/1/state/state_test.go#newcode1438
state/state_test.go:1438: tag: "service-ser-vice2",
We need a relation tag test here.

https://codereview.appspot.com/12748044/

« Back to merge proposal