-
-
Notifications
You must be signed in to change notification settings - Fork 958
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
Update Keyring API reference #1170
Conversation
Preview published: 1165-keyring-reference |
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. Just did a rough review, but can confirm links work, it's typo-free, and has constant formatting
I find the name Keyring Interface API a bit unclear, it doesn't pass the idea that it contains chain-specific methods that a Snap can choose to support. Maybe we could rename the APIs like:
This is only a suggestion and I would like to hear what others have to say (ping @gantunesr @montelaidev). |
Preview published: 1165-keyring-reference |
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.
A small nit
I agree with @danroc opinion, also seeing at these descriptions,
And what is specified in the
I see an opportunity to consolidated these definitions and push for the renaming as proposed here |
Preview published: 1165-keyring-reference |
Removed dependencies detected. Learn more about Socket for GitHub ↗︎ 🚮 Removed packages: npm/ansi-sequence-parser@1.1.1, npm/docusaurus-plugin-typedoc@1.0.0-next.17, npm/lunr@2.3.9, npm/marked@4.3.0, npm/shiki@0.14.7, npm/typedoc-plugin-markdown@4.0.0-next.22, npm/typedoc@0.25.9, npm/vscode-oniguruma@1.7.0, npm/vscode-textmate@8.0.0 |
Preview published: 1165-keyring-reference |
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, thanks!
This removes the
external/keyring-api
submodule and creates the Keyring API reference manually, adding the categories "Keyring Client API" and "Keyring Interface API." It also updates related content. Fixes #1165.Previews:
For the Client API reference, I used this Notion page for reference and checked it against the updated source code.