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

Issue 13490043: various: Relation tags use keys, not ids (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by dimitern
Modified:
10 years, 7 months ago
Reviewers:
jameinel, mp+183634
Visibility:
Public.

Description

various: Relation tags use keys, not ids This changes the format of relation tags from "relation-id" to "relation-key", where the key has the format "svc1.rel1#svc2.rel2". Changed the relevant bits in names/ and state/ as needed. https://code.launchpad.net/~dimitern/juju-core/117-relation-tags-use-keys/+merge/183634 (do not edit description out of merge proposal)

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+102 lines, -85 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M names/relation.go View 1 chunk +25 lines, -10 lines 0 comments Download
M names/relation_test.go View 1 chunk +22 lines, -9 lines 0 comments Download
M names/tag.go View 1 chunk +3 lines, -2 lines 0 comments Download
M names/tag_test.go View 2 chunks +3 lines, -3 lines 0 comments Download
M state/apiserver/uniter/uniter.go View 2 chunks +5 lines, -13 lines 0 comments Download
M state/apiserver/uniter/uniter_test.go View 9 chunks +36 lines, -36 lines 0 comments Download
M state/relation.go View 1 chunk +1 line, -1 line 0 comments Download
M state/state.go View 2 chunks +1 line, -7 lines 0 comments Download
M state/state_test.go View 3 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 2
dimitern
Please take a look.
10 years, 7 months ago (2013-09-03 11:52:53 UTC) #1
jameinel
10 years, 7 months ago (2013-09-03 12:56:46 UTC) #2
The actual code is all LGTM.

I don't actually prefer the structure of the keys, it seems like we are adding
complexity because we might someday use them as filenames (though we don't
actually ever do that today).

But if you have buy in from other people about the structure of the key, it
isn't a particularly big deal.
Sign in to reply to this message.

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