Skip to content

Conversation

@ChrisChinchilla
Copy link

@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 <[email protected]>
@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 <[email protected]>
@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 <[email protected]>

# 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
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

Chris Chinchilla and others added 4 commits December 18, 2023 11:04
Signed-off-by: Chris Chinchilla <[email protected]>
…hub.com:KILTprotocol/docs into git-checkout-2801-3⃣-kilt-ext-lib-documentation
Signed-off-by: Chris Chinchilla <[email protected]>
@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
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 <[email protected]>
@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
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 <[email protected]>
@Ad96el
Copy link
Member

Ad96el commented Jan 8, 2024

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

@ChrisChinchilla
Copy link
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.

5 participants