-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework the target to be a root capability and rename capabilityGrant to capabilityDelegate #18
Conversation
Part of #13. One more bit to doin reworking some text though.
…an ocap. - Remove all references of invocationTarget, replacing with parentCapability property or simply the concept of the target. - Also rework invocation explaination to be accurate to this new text and clearer. - Replace last reference of capabilityGrant with capabilityDelegate. Closes #14 and #13.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly minor nits... one question about language tense used in proofPurpose... suggesting we use words ending in "tion"... words used to indicate an action is being taken, which is what an inovcation is. The other "proofPurpose" suggestion would be "capabilityInvocation" (instead of delegation)?
index.html
Outdated
"proof": { | ||
"type": "RsaSignature2016", | ||
"proofPurpose": "capabilityGrant", | ||
"proofPurpose": "capabilityDelegate", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not capabilityDelegation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no problem with that. I'll update it.
index.html
Outdated
@@ -421,6 +421,12 @@ <h2>Terminology</h2> | |||
(linked data) object. | |||
</dd> | |||
|
|||
<dt>target</dt> | |||
<dd> | |||
The entity that a capability, upon being invoked, permits the authority |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This language feels more complex than it needs to be. Why not "The entity that will be acted upon when the capability is invoked." ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, will update.
index.html
Outdated
<p> | ||
Every capability document MUST have: | ||
Every capability document, excepting the target, MUST have: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"excepting"? or "except for" ... suggest using simpler language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
index.html
Outdated
<!-- TODO: Should this be by any of the authentication material which | ||
has authority to use this capability, or only the nearest | ||
invoker on the chain? --> | ||
</li> | ||
</ul> | ||
|
||
<p> | ||
"Top-level" capabilities, which do not have a | ||
<code>parentCapability</code> property, MUST have: | ||
Every capability document, excepting the target, MAY have: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest changing "excepting" to "except for"...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
@@ -421,6 +421,11 @@ <h2>Terminology</h2> | |||
(linked data) object. | |||
</dd> | |||
|
|||
<dt>target</dt> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this renamed from "invocationTarget" to just "target"? I think it's more likely to conflict with other vocabularies making plain JSON implementations more difficult.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no invocationTarget or any such target property anymore: since the target is the root capability, we now use parentCapability to point to it. So this just refers to the concept of a target, not a target property.
Maybe there's a way to make that clearer? What do you think?
These turned out to be somewhat intertwined so done in one pass but across two commits.