-
Notifications
You must be signed in to change notification settings - Fork 99
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
Amplify/broaden def of 'verification method' #284
Conversation
Signed-off-by: Daniel Hardman <[email protected]>
Signed-off-by: Daniel Hardman <[email protected]>
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.
Looks good. I believe this also addresses #57.
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 is awesome. Only improvement I can see is to the bottom paragraph.
We have a couple scenarios bundled here... threshold signatures and biometrics...
I would like to see concrete example of both somewhere, if we are going to comment on them here... but I am also not aware that there are any, so this might not be possible.
The only aspect of this PR I find problematic is that it adds a lot of text to the Terminology section. All other terms in that section consist of a single paragraph, whereas this one has three. Perhaps most of this new text should be moved elsewhere (especially if a new |
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.
LGTM other than minor editorial nits to not use normative looking language (someone on the Internet inevitably complains about it while we're in the final push to REC and the Editors have to go through and remove all lowercase must, may, should, etc. from the spec).
I also note @peacekeeper's concern, and that we'll probably create a new section on verification methods and move some of the text to that section, with a pointer from the definition on those that want to learn more. I think we should preserve @dhh1128's text through to the end because it does the best job of describing what a verification method is to date.
Thank you, @dhh1128! :)
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.
Editorial, multiple positive reviews, changes made, no objections, merging.
Signed-off-by: Daniel Hardman [email protected]
See #276