Skip to content
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

Extension library docs #289

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ChrisChinchilla
Copy link
Contributor

@ChrisChinchilla ChrisChinchilla commented Dec 5, 2023

fixes KILTProtocol/ticket#2801

Moves Extention library read me to documentation.

Checklist:

  • I have verified that the code works
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)
    • Either PR or Ticket to update the Docs
    • Link the PR/Ticket here

@ChrisChinchilla ChrisChinchilla force-pushed the git-checkout-2801-3⃣-kilt-ext-lib-documentation branch from 557bfa4 to 18b9394 Compare December 12, 2023 13:16
Signed-off-by: Chris Chinchilla <chris@kilt.io>
@ChrisChinchilla ChrisChinchilla force-pushed the git-checkout-2801-3⃣-kilt-ext-lib-documentation branch from 18b9394 to c2668d1 Compare December 12, 2023 13:17
Signed-off-by: Chris Chinchilla <chris@kilt.io>
@ChrisChinchilla ChrisChinchilla force-pushed the git-checkout-2801-3⃣-kilt-ext-lib-documentation branch from ca12d81 to d05a168 Compare December 14, 2023 10:38
…hub.com:KILTprotocol/docs into git-checkout-2801-3⃣-kilt-ext-lib-documentation

Signed-off-by: Chris Chinchilla <chris@kilt.io>

# Conflicts:
#	docs/develop/01_sdk/04_integrate/04_extension_api.md
@ChrisChinchilla ChrisChinchilla force-pushed the git-checkout-2801-3⃣-kilt-ext-lib-documentation branch from d05a168 to 4871cbd Compare December 14, 2023 10:45
@ChrisChinchilla
Copy link
Contributor Author

Link checker is breaking and I'm not sure why

Copy link
Contributor

@rflechtner rflechtner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall! We should probably try, test, and publish the library before we publish the docs, right?
Also I'm wondering whether it makes sense to introduce a clear distinction between functions intended for application developers and those targeting wallet (extension) implementers

docs/develop/01_sdk/04_integrate/04_extension_api.md Outdated Show resolved Hide resolved
docs/develop/01_sdk/04_integrate/04_extension_api.md Outdated Show resolved Hide resolved
docs/develop/01_sdk/04_integrate/03_distillery.md Outdated Show resolved Hide resolved
ChrisChinchilla and others added 4 commits December 18, 2023 11:04
Co-authored-by: Raphael Flechtner <39338561+rflechtner@users.noreply.github.com>
Signed-off-by: Chris Chinchilla <chris@kilt.io>
…hub.com:KILTprotocol/docs into git-checkout-2801-3⃣-kilt-ext-lib-documentation
Signed-off-by: Chris Chinchilla <chris@kilt.io>
@ChrisChinchilla ChrisChinchilla force-pushed the git-checkout-2801-3⃣-kilt-ext-lib-documentation branch from 0669157 to a8bbb9b Compare December 19, 2023 12:08
@ChrisChinchilla
Copy link
Contributor Author

@rflechtner Made some changes…

I'm wondering whether it makes sense to introduce a clear distinction between functions intended for application developers and those targeting wallet (extension) implementers

Are you saying there's probably more that needs documenting in addition to what's here already?

Signed-off-by: Chris Chinchilla <chris@kilt.io>
@rflechtner
Copy link
Contributor

Are you saying there's probably more that needs documenting in addition to what's here already?

What I'm hinting at is that different functions are meant for use in different environments; verifyDidConfigResource for example is something that would be called by a browser extension, createCredential and didConfigResourceFromCredential are meant to be used in a backend implementation, and initializeKiltExtensionAPI´ & getExtensions` etc. would be called in the frontend. We haven't pointed that out anywhere, and it could help with understanding the use and purpose of this library.

@Ad96el
Copy link
Member

Ad96el commented Dec 21, 2023

LGTM in general. I'm just wondering why the messaging part is missing. Messaging is crucial for interacting with the extension. Is it not the scope of this PR?

@ChrisChinchilla
Copy link
Contributor Author

@Ad96el It was never in the docs I took over, I kind of knew that there were methods missing. If you say it's essential, then we can add before merging.

Signed-off-by: Chris Chinchilla <chris@kilt.io>
@Ad96el
Copy link
Member

Ad96el commented Jan 8, 2024

Seems like I forgot to update the README. 😕
Will tackle it today.

@ChrisChinchilla
Copy link
Contributor Author

@Ad96el This read me? https://github.com/KILTprotocol/kilt-extension-api/blob/main/README.md

I think we're stuck in a dependency loop as we can't add a link until this is merged and we can't merge this until there's a release. I'm not sure if the extension library is that high-traffic, so we can probably leave it for now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants