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

Issue 5541054: Subordinate Services Spec (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 8 months ago by bcsaller
Modified:
9 years, 6 months ago
Reviewers:
hazmat, niemeyer, mp+88515, rog
Visibility:
Public.

Description

Based on input from the week of the rally here are the proposed changes to the subordinates specification. This (final) revision contains two storage containment models that differ only in that container relations nest the traditional ZK relation structure under the id of the container its child nodes interact under. https://code.launchpad.net/~bcsaller/juju/subordinate-spec/+merge/88515 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 51

Patch Set 2 : Subordinate Services Spec #

Total comments: 82

Patch Set 3 : Subordinate Services Spec #

Total comments: 14

Patch Set 4 : Subordinate Services Spec #

Total comments: 12

Patch Set 5 : Subordinate Services Spec #

Patch Set 6 : Subordinate Services Spec #

Total comments: 15

Patch Set 7 : Subordinate Services Spec #

Total comments: 4

Patch Set 8 : Subordinate Services Spec #

Total comments: 8

Patch Set 9 : Subordinate Services Spec #

Total comments: 12

Patch Set 10 : Subordinate Services Spec #

Unified diffs Side-by-side diffs Delta from patch set Stats (+454 lines, -2 lines) Patch
M .bzrignore View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M docs/source/charm.rst View 1 2 3 4 5 6 7 8 9 1 chunk +14 lines, -0 lines 0 comments Download
M docs/source/charm-upgrades.rst View 1 2 3 4 5 6 7 8 9 2 chunks +1 line, -2 lines 0 comments Download
A docs/source/drafts/implicit-relations.rst View 1 2 3 4 5 6 7 8 9 1 chunk +61 lines, -0 lines 0 comments Download
A docs/source/drafts/subordinate-internals.rst View 1 2 3 4 5 6 7 8 9 1 chunk +201 lines, -0 lines 0 comments Download
A docs/source/drafts/subordinate-services.rst View 1 2 3 4 5 6 7 8 9 1 chunk +176 lines, -0 lines 0 comments Download

Messages

Total messages: 29
bcsaller
Please take a look.
9 years, 8 months ago (2012-01-13 15:14:17 UTC) #1
rog
a few initial minor comments on the implicit-interfaces doc. https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst File docs/source/drafts/implicit-interfaces.rst (right): https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst#newcode4 docs/source/drafts/implicit-interfaces.rst:4: ...
9 years, 8 months ago (2012-01-13 15:58:22 UTC) #2
bcsaller
https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst File docs/source/drafts/implicit-interfaces.rst (right): https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst#newcode4 docs/source/drafts/implicit-interfaces.rst:4: Juju has the ability to provide specialized interfaces to ...
9 years, 8 months ago (2012-01-13 16:06:15 UTC) #3
hazmat
Looks good, the implicit-interfaces doc could use a once over, and i wouldn't mind chatting ...
9 years, 8 months ago (2012-01-18 18:32:15 UTC) #4
bcsaller
ptal https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst File docs/source/drafts/implicit-interfaces.rst (right): https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst#newcode7 docs/source/drafts/implicit-interfaces.rst:7: interested services to gather broad lifecycle oriented events ...
9 years, 8 months ago (2012-01-19 09:00:01 UTC) #5
bcsaller
Please take a look.
9 years, 8 months ago (2012-01-19 10:13:05 UTC) #6
niemeyer
https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst File docs/source/drafts/implicit-interfaces.rst (right): https://codereview.appspot.com/5541054/diff/1/docs/source/drafts/implicit-interfaces.rst#newcode6 docs/source/drafts/implicit-interfaces.rst:6: on the part of the charm author. Implicit interface ...
9 years, 8 months ago (2012-01-19 15:06:18 UTC) #7
bcsaller
https://codereview.appspot.com/5541054/diff/9001/docs/source/drafts/implicit-interfaces.rst File docs/source/drafts/implicit-interfaces.rst (right): https://codereview.appspot.com/5541054/diff/9001/docs/source/drafts/implicit-interfaces.rst#newcode7 docs/source/drafts/implicit-interfaces.rst:7: author. On 2012/01/19 15:06:18, niemeyer wrote: > s/author./author of ...
9 years, 8 months ago (2012-01-22 14:27:41 UTC) #8
bcsaller
Please take a look.
9 years, 8 months ago (2012-01-22 15:20:35 UTC) #9
niemeyer
Good changes overall, but you've missed comments for a whole file in the review (the ...
9 years, 8 months ago (2012-01-23 14:49:41 UTC) #10
bcsaller
Please take a look.
9 years, 8 months ago (2012-01-26 19:51:19 UTC) #11
bcsaller
forgot to mail this on the last lbox propose https://codereview.appspot.com/5541054/diff/8003/docs/source/drafts/implicit-relations.rst File docs/source/drafts/implicit-relations.rst (right): https://codereview.appspot.com/5541054/diff/8003/docs/source/drafts/implicit-relations.rst#newcode28 docs/source/drafts/implicit-relations.rst:28: ...
9 years, 8 months ago (2012-01-26 21:01:19 UTC) #12
niemeyer
https://codereview.appspot.com/5541054/diff/8003/docs/source/drafts/implicit-relations.rst File docs/source/drafts/implicit-relations.rst (right): https://codereview.appspot.com/5541054/diff/8003/docs/source/drafts/implicit-relations.rst#newcode17 docs/source/drafts/implicit-relations.rst:17: `juju-info`, specified as :: On 2012/01/23 14:49:41, niemeyer wrote: ...
9 years, 7 months ago (2012-02-06 19:46:19 UTC) #13
bcsaller
Please take a look.
9 years, 7 months ago (2012-02-07 07:54:04 UTC) #14
bcsaller
https://codereview.appspot.com/5541054/diff/15003/docs/source/charm.rst File docs/source/charm.rst (right): https://codereview.appspot.com/5541054/diff/15003/docs/source/charm.rst#newcode89 docs/source/charm.rst:89: :doc:`subordinate-services`. On 2012/02/06 19:46:19, niemeyer wrote: > Very nice, ...
9 years, 7 months ago (2012-02-07 08:14:03 UTC) #15
bcsaller
Please take a look.
9 years, 7 months ago (2012-02-09 03:40:06 UTC) #16
niemeyer
https://codereview.appspot.com/5541054/diff/31002/docs/source/drafts/implicit-relations.rst File docs/source/drafts/implicit-relations.rst (right): https://codereview.appspot.com/5541054/diff/31002/docs/source/drafts/implicit-relations.rst#newcode59 docs/source/drafts/implicit-relations.rst:59: provides less information) `juju-info` interface. This is the opposite ...
9 years, 7 months ago (2012-02-10 00:20:39 UTC) #17
bcsaller
https://codereview.appspot.com/5541054/diff/31002/docs/source/drafts/implicit-relations.rst File docs/source/drafts/implicit-relations.rst (right): https://codereview.appspot.com/5541054/diff/31002/docs/source/drafts/implicit-relations.rst#newcode59 docs/source/drafts/implicit-relations.rst:59: provides less information) `juju-info` interface. There was some confusion ...
9 years, 7 months ago (2012-02-10 01:32:25 UTC) #18
bcsaller
Please take a look.
9 years, 7 months ago (2012-02-10 01:33:19 UTC) #19
niemeyer
Thanks a lot for the extensive spec, and for keeping up with it Ben. Just ...
9 years, 7 months ago (2012-02-10 18:28:30 UTC) #20
bcsaller
https://codereview.appspot.com/5541054/diff/33002/docs/source/drafts/subordinate-internals.rst File docs/source/drafts/subordinate-internals.rst (right): https://codereview.appspot.com/5541054/diff/33002/docs/source/drafts/subordinate-internals.rst#newcode68 docs/source/drafts/subordinate-internals.rst:68: handled automatically. On 2012/02/10 18:28:30, niemeyer wrote: > Please ...
9 years, 7 months ago (2012-02-10 18:32:49 UTC) #21
bcsaller
Please take a look.
9 years, 7 months ago (2012-02-27 19:31:42 UTC) #22
niemeyer
https://codereview.appspot.com/5541054/diff/39001/docs/source/drafts/subordinate-internals.rst File docs/source/drafts/subordinate-internals.rst (right): https://codereview.appspot.com/5541054/diff/39001/docs/source/drafts/subordinate-internals.rst#newcode30 docs/source/drafts/subordinate-internals.rst:30: container/ As discussed online: 1) Drop container/ https://codereview.appspot.com/5541054/diff/39001/docs/source/drafts/subordinate-internals.rst#newcode41 docs/source/drafts/subordinate-internals.rst:41: ...
9 years, 7 months ago (2012-02-27 20:24:19 UTC) #23
bcsaller
Please take a look.
9 years, 7 months ago (2012-02-27 23:13:59 UTC) #24
hazmat
lgtm
9 years, 7 months ago (2012-02-28 00:04:54 UTC) #25
niemeyer
https://codereview.appspot.com/5541054/diff/39001/docs/source/drafts/subordinate-internals.rst File docs/source/drafts/subordinate-internals.rst (right): https://codereview.appspot.com/5541054/diff/39001/docs/source/drafts/subordinate-internals.rst#newcode92 docs/source/drafts/subordinate-internals.rst:92: interface: name On 2012/02/27 20:24:19, niemeyer wrote: > s/name/<name>/, ...
9 years, 7 months ago (2012-02-28 00:45:20 UTC) #26
bcsaller
Please take a look.
9 years, 7 months ago (2012-02-28 01:13:59 UTC) #27
bcsaller
Thanks for looking this over again, believe I got it all. https://codereview.appspot.com/5541054/diff/39003/docs/source/drafts/subordinate-internals.rst File docs/source/drafts/subordinate-internals.rst (right): ...
9 years, 7 months ago (2012-02-28 01:14:08 UTC) #28
niemeyer
9 years, 7 months ago (2012-02-28 01:24:19 UTC) #29
LGTM, thanks.
Sign in to reply to this message.

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