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

Make loadOrCreate accept generate options #101

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

Powersource
Copy link
Contributor

@Powersource Powersource commented Oct 13, 2022

I'm working on ssbc/ssb-tribes2#10 and to do that I need to adapt the tests there. And those tests create their keys with loadOrCreate.

  • update docs

@Powersource
Copy link
Contributor Author

@staltz @mixmix

Copy link
Member

@mixmix mixmix left a comment

Choose a reason for hiding this comment

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

From my read of this this is a non-breaking change right?

Seems good to me (reviewed on phone)

@Powersource
Copy link
Contributor Author

Yep should be non-breaking

@Powersource
Copy link
Contributor Author

@mixmix @staltz i can't merge, do you want to do that and publish a new version?

@staltz
Copy link
Member

staltz commented Oct 14, 2022

As said on the tribes2 PR, I think this change is quite deep in the SSB stack and breaks a strong (legacy) assumption that classic feed format is used everywhere. E.g. there are places where we convert from a multiserver/shs address to SSB ID, by plucking the public base64 and adding @ in front and .ed25519 in the end.

So could we put this PR on hold or close it?

@Powersource
Copy link
Contributor Author

@staltz why not give the user the option at least? And if not the feed format, this will allow the user to set the key seed too.

@staltz
Copy link
Member

staltz commented Oct 14, 2022

Good point

@mixmix
Copy link
Member

mixmix commented Feb 13, 2023

If this is a non-breaking change @Powersource I recommend we merge + publish
Will make sure this is added to npm org

@Powersource Powersource merged commit 27b2fa0 into ssbc:main Feb 15, 2023
@Powersource Powersource deleted the load-or-create-sync-buttwoo branch February 15, 2023 12:24
@Powersource
Copy link
Contributor Author

@mixmix merged now. will you publish?

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