-
Notifications
You must be signed in to change notification settings - Fork 27
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
fix!: update examples for new authorize flow #474
Conversation
unfortunately this also required some changes to the "keyring" interfaces - I'd like to ship these as a minor version since imho we didn't "officially" support the cancelSpaceRegistration function in this release and I would want people to be automatically updated to this version as soon as possible.
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f3a292b:
|
various fixes to get them running
*/ | ||
registerSpace: (email: string) => Promise<void> | ||
/** | ||
* Abort an ongoing account registration. | ||
*/ | ||
cancelRegisterSpace: () => void, | ||
cancelAuthorize: () => void, |
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.
This would be backward incompatible, but the PR title does not have a feat!
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.
yeaaaa I think it was a bug in this major release that this wasn't changed (see PR description), so I'd like it to ship in a minor release.
I know it's technically semver heresy, but I also think it was just a bug that this didn't break with the last major
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.
put another way - if someone has already gone through the last major version upgrade but is depending on this function, I'd prefer to have their code break when they update the minor rather than require them to do another major version upgrade here, so my preference is to ship this in a minor
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.
ok we chatted in person and I've moved over to "let's just ship a major release and stop worrying about this" - should only result in a bit of extra work for me in any case, and even that will be pretty minor
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.
Would still be helpful to get @alanshaw's brain on this one - @gobengo and I went back and forth but ultimately decided just shipping a new major release is probably the least ugly thing to do here, rather than shipping a minor release with a backwards incompatible change.
Probably a moot point given the current level of outside adoption of these libraries, but worth being on the same page about regardless.
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.
I think it's fine to release as major.
More generally (not for this PR), i know that authorizing is now the step that can take a while (let's fix that too!) but I think all our methods that do a fetch should be cancellable in some way... like we should allow callers to pass in an AbortSignal
so they can give up if their UI wants to.... any call to our api could stall.
🤖 I have created a release *beep* *boop* --- ## [4.0.0](keyring-core-v3.0.0...keyring-core-v4.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([#474](#474)) ### Bug Fixes * update examples for new authorize flow ([#474](#474)) ([0925c21](0925c21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [5.0.0](react-keyring-v4.0.0...react-keyring-v5.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([#474](#474)) ### Bug Fixes * update examples for new authorize flow ([#474](#474)) ([0925c21](0925c21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [4.0.0](vue-keyring-v3.0.0...vue-keyring-v4.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([#474](#474)) ### Bug Fixes * update examples for new authorize flow ([#474](#474)) ([0925c21](0925c21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [4.0.0](solid-keyring-v3.0.0...solid-keyring-v4.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([#474](#474)) ### Bug Fixes * update examples for new authorize flow ([#474](#474)) ([0925c21](0925c21)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [4.0.0](storacha/w3ui@keyring-core-v3.0.0...keyring-core-v4.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ### Bug Fixes * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ([19dc158](storacha/w3ui@19dc158)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [5.0.0](storacha/w3ui@react-keyring-v4.0.0...react-keyring-v5.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ### Bug Fixes * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ([19dc158](storacha/w3ui@19dc158)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [4.0.0](storacha/w3ui@vue-keyring-v3.0.0...vue-keyring-v4.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ### Bug Fixes * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ([19dc158](storacha/w3ui@19dc158)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [4.0.0](storacha/w3ui@solid-keyring-v3.0.0...solid-keyring-v4.0.0) (2023-03-30) ### ⚠ BREAKING CHANGES * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ### Bug Fixes * update examples for new authorize flow ([storacha#474](storacha/w3ui#474)) ([19dc158](storacha/w3ui@19dc158)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Travis Vachon <[email protected]>
Unfortunately this also required some changes to the "keyring" interfaces - shipping another major release based on these changes is slightly annoying but probably the right thing to do, and honestly I'm probably the only person who will need to upgrade deps for now.