Skip to content
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

Re-order actions you can take with a Result #201

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

illicitonion
Copy link
Contributor

It's generally better practice to handle or propagate errors, rather than panicking in response to them. This edit moves panicking to be the last option introduced, rather than the first. It also adds caveats to avoid doing so, and explicitly mentions propagating as something to consider.

It's generally better practice to handle or propagate errors, rather
than panicking in response to them. This edit moves panicking to be the
_last_ option introduced, rather than the first. It also adds caveats to
avoid doing so, and explicitly mentions propagating as something to
consider.
Copy link

netlify bot commented Oct 31, 2024

Deploy Preview for taupe-lily-3539f6 ready!

Name Link
🔨 Latest commit 2e2f6b9
🔍 Latest deploy log https://app.netlify.com/sites/taupe-lily-3539f6/deploys/67238ad3f2757d0008ee3617
😎 Deploy Preview https://deploy-preview-201--taupe-lily-3539f6.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@hdoordt
Copy link
Member

hdoordt commented Jan 27, 2025

@illicitonion Thanks for your contribution! I think it's a good addition overall. However, the story about the ?-operator is a bit more complicated, as it will do a conversion as well if possible and necessary. Can you update your text so that it covers that part as well?

@LukeMathWalker
Copy link
Collaborator

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 ? actually behaves.

My suggestion is to remove ? from this bullet list, make it its own sub-section in the same chapter and clarify in a footnote that there's more to this that'll be discussed later, with a link to the chapter where we do. That chapter's text also needs to be adjusted to reflect the fact that ? has already been introduced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants