-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat!: make functions return Result and implement Node Killer #1067
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.
Thanks for the nice refactor and devX improvement!
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 going to break every test out there. It should definitely be fixed while we have the time, my thinking is just whether we want to make some more changes to this so that we may at least have something more to show for it besides better panic handling.
For example, one thing we've been putting off for so long is the ability to kill the spawned node. Useful when users want to test the behavior of their app when the node is down.
Currently, they need to kill the tokio runtime since a handle was hidden somewhere inside that keeps the node alive.
Ideally, we'd ship this out with that feature (and maybe any other pressing ones) and at least make a feature-rich-breaking change.
@FuelLabs/sdk-rust
I could do it in this PR, but I think it is better to make a new PR. I will do as @FuelLabs/sdk-rust decides. Anyway, I'm going to start implementing |
…s into Salka1988/return_result
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 like the way this looks now! I have some change requests though...
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
Co-authored-by: hal3e <[email protected]>
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.
Great work!
Refactored functions to return Result for better error handling.
I've implemented FuelService and added support for the fuel_core_services::Service trait. This allows us to have finer control over the behavior of FuelNode;
Checklist