-
Notifications
You must be signed in to change notification settings - Fork 18
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
VDX-88 #54
VDX-88 #54
Conversation
plugin.schema.json is supplied as we cannot let it generate one as callbacks are being used |
the build failing is because of a test in the ms-authenticator that takes to long to complete |
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.
Basically some small remarks about some documentation and naming
packages/wellknown-did-verifier/__tests__/shared/wellKnownDidVerifierAgentLogic.ts
Outdated
Show resolved
Hide resolved
packages/wellknown-did-verifier/src/types/IWellKnownDidVerifier.ts
Outdated
Show resolved
Hide resolved
} | ||
|
||
export interface IRegisterSignatureVerificationArgs { | ||
key: string |
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.
Can we change the name. This might be confusing with did private/public keys
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.
renamed
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.
See other remarks. It can still be confused with a public key for signature verification. Suggest to name it to something like callbackName, which makes immediate sense to a developer
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.
renamed
@@ -24,12 +20,12 @@ export interface IWellKnownDidVerifierOptionsArgs { | |||
} | |||
|
|||
export interface IRegisterSignatureVerificationArgs { | |||
key: string | |||
signatureVerificationKey: string |
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.
callbackName might be better, because this can still be confused with a public key for signature verification
signatureVerification: (args: IVerifyCallbackArgs) => Promise<IVerifyCredentialResult> | ||
} | ||
|
||
export interface IRemoveSignatureVerificationArgs { | ||
key: string | ||
signatureVerificationKey: string |
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.
callbackName might be better, because this can still be confused with a public key for signature verification
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.
renamed
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
No description provided.