-
Notifications
You must be signed in to change notification settings - Fork 12
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
chore: add auth-service snippets #17
chore: add auth-service snippets #17
Conversation
00ef8fd
to
5e98ebd
Compare
re: KeyHandleId. String is fine for now. Couldn't find the format of a Key Handle but it'd be nice if we had pattern matching and a explicit |
@@ -0,0 +1,28 @@ | |||
title: UpdateCredentialRequest |
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.
Really confused about this definition. It doesn't be seem to be a request but more akin to a property used in ConsentIDPutRequest
https://github.com/mojaloop/api-snippets/pull/17/files#diff-2b332d878ba42f73f319a12fb3fccfb4R22 .
Maybe something like CredentialProof? @Auth-Team
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 think this was my fault - and there's a good chance I did something wrong here.
One of the challenges/quirks with the api design is that we have a ton of fields which could be null or undefined. My goal with request bodies (or parts of request bodies) was to have a schema which would be very explicit, and thus make writing the types and code much easier.
Renaming it from ...Request
would make this less confusing - CredentialProof
seems suitable, or perhaps UpdatedCredential
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.
UpdatedCredential sounds good. done.
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
Decided to do auth-service as well since it gives me a better view of the api definition as a whole.
No
/docs
file generated yet. Need to merge thirdparty-api-adapter changes first so I can make changes in order.