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

Update README.md with simple example and ready() #291

Merged
merged 6 commits into from
Nov 18, 2024

Conversation

daidoji
Copy link
Contributor

@daidoji daidoji commented Nov 16, 2024

Added simple code example to get started and a note about calling the ready() function for the library to be usable.

Added simple code example to get started and a  note about calling the ready() function for the library to be usable.
Copy link

codecov bot commented Nov 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.59%. Comparing base (c13b6e8) to head (0a7b506).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #291   +/-   ##
=======================================
  Coverage   83.59%   83.59%           
=======================================
  Files          48       48           
  Lines        4225     4225           
  Branches     1029     1055   +26     
=======================================
  Hits         3532     3532           
+ Misses        689      663   -26     
- Partials        4       30   +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

README.md Outdated

await ready();

const bran = '0'.repeat(21);
Copy link
Contributor

@iFergal iFergal Nov 18, 2024

Choose a reason for hiding this comment

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

We should probably use randomPasscode here, just so new users are aware Signify can generate it instead of external

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Why is it a shared secret with KERIA though? It's generated locally with libsodium

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iFergal isn't this meant to be stored OOB by the user to access/manipulate state on KERIA?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah since Signify is stateless but it's never shared with KERIA. It's just used to derive the keys locally, and those public keys/events etc are shared with KERIA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iFergal yeah you're right, I'll remove the comment.

randomPasscode added to example to make new users aware.
removed erroneous comment.
daidoji added a commit to daidoji/signify-ts that referenced this pull request Nov 18, 2024
Happened to get
https://github.com/WebOfTrust/signify-ts/actions/runs/11895128350/job/33143979521?pr=291
when updating README in WebOfTrust#291.

This PR fixes that vulnerability in cross-spawn so that test should pass
now.
@daidoji
Copy link
Contributor Author

daidoji commented Nov 18, 2024

Added #292 to fix failing test action in this PR.

lenkan pushed a commit that referenced this pull request Nov 18, 2024
Happened to get
https://github.com/WebOfTrust/signify-ts/actions/runs/11895128350/job/33143979521?pr=291
when updating README in #291.

This PR fixes that vulnerability in cross-spawn so that test should pass
now.
@lenkan lenkan merged commit 2bf0df1 into WebOfTrust:main Nov 18, 2024
8 checks passed
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.

3 participants