-
Notifications
You must be signed in to change notification settings - Fork 5
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
Implement Sessions #21
Conversation
This is exciting! 🥳 |
3898769
to
8283256
Compare
bd70297
to
9eeb920
Compare
@fancycade This should be god to go. I've opened #24 and #25 and #26 based on things that I noticed when implementing this. |
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.
Looks good! This was quite the effort!
The c code is similar to how accounts was done so I think it is okay.
Didn't notice any misspellings.
Re #24 >
I think returning tuples makes sense. If there is a chance to error then I think tuple is good, otherwise you can return the value because there is no way it could be anything else.
My suggestion would be to do a quick release, then try to integrate it with the SDK and see how you feel.
Closes #12.