- Notifications
You must be signed in to change notification settings - Fork1.8k
Re-order actions you can take with a Result#201
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
It's generally better practice to handle or propagate errors, ratherthan panicking in response to them. This edit moves panicking to be the_last_ option introduced, rather than the first. It also adds caveats toavoid doing so, and explicitly mentions propagating as something toconsider.
netlifybot commentedOct 31, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortaupe-lily-3539f6 ready!
To edit notification comments on pull requests, go to yourNetlify site configuration. |
hdoordt commentedJan 27, 2025
@illicitonion Thanks for your contribution! I think it's a good addition overall. However, the story about the |
LukeMathWalker commentedJan 28, 2025
We should be careful here with respect to when concepts are introduced—we have no knowledge of conversion traits at this stage, so we can't give a precise explanation as to how My suggestion is to remove |
It's generally better practice to handle or propagate errors, rather than panicking in response to them. This edit moves panicking to be thelast option introduced, rather than the first. It also adds caveats to avoid doing so, and explicitly mentions propagating as something to consider.