-
Notifications
You must be signed in to change notification settings - Fork 284
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
Return Results from native function wrappers #1065
Return Results from native function wrappers #1065
Conversation
ace915e
to
29b92e7
Compare
Woah! This is awesome. Thanks for taking this on. I notice a lot of |
It serves the purpose of indicating that there's no additional information like a boolean success value, that needs to be handled. |
I should note that it's not required, and I'm happy to remove the |
I think I would prefer removing it. It feels like a technique that would make more sense back before the |
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 is a great improvement, thank you for doing this!
4ce3d95
to
ad2cb8f
Compare
Rebased on latest |
Merged, thank you so much for tackling this! |
This doesn't bubble the
Result
return type all the way to the top, and it uses the existing status code as the error type instead of defining a new type as suggested in #802 (comment).Progress on #802