-
Notifications
You must be signed in to change notification settings - Fork 86
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 with @stacks.js/* and various minor improvements #57
Conversation
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.
LGTM
@kyranjamie Mind reviewing again with particular focus on the |
I can, though following discussion here, maybe we want to wait before merging this if we're going to change the packaging structures so that connect exports auth, rather than this way around. |
I'd prefer not to make that packages discussion blocking here since the related documentation is currently broken and need these changes urgently to fix them. |
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.
LGTM @markmhx
I'd be inclined to rename src/stacks.js
to stacks-auth
or something to be a bit more descriptive what it contains, otherwise looks clean.
@kyranjamie I've improved the relevant |
All good |
@stacks/auth
instead of@blockstack/connect
stacks
moduleThis PR resolves / supplants #53 and is needed mainly to update content found at https://docs.blockstack.org/authentication/building-todo-app to see how Stacks.js is used to integrate authentication.