-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
chore(stdlib)!: Replace parseInt
error strings with structured error enum
#1755
chore(stdlib)!: Replace parseInt
error strings with structured error enum
#1755
Conversation
2c2d161
to
4ceecb2
Compare
This is now blocked on #1886 |
@spotandjake is this unblocked now that the exception re-export PR has been merged? |
No this is still blocked. We had decided on a conversation in discord, to use an enum instead of an exception. #1886 is what is blocking this at the moment. |
4ceecb2
to
8ba7ea0
Compare
@spotandjake I don't remember that conversation exactly. We don't really want many enums in pervasives because they can cause ambiguous type inference with user code. |
8db79a6
to
71f0002
Compare
Switched back to an |
4f9a822
to
ff26f8d
Compare
When trying to graindoc this I get the error |
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 has been switched to an enum
now.
parseInt
use an exception in the err caseparseInt
error strings with structured error enum
100e879
to
1090422
Compare
This makes
parseInt
use an exception in theerr
case.On a note though until #1053 is fixed I dont know if it makes sense to make this change as you lose the ability to pattern match on the individual cases without importing the exceptions directly from the runtime which we dont want users doing.
That could still make it in before 0.6 so this pr is up for that and also just in case we want to move forward on this anyways.
This has been blocked by: #1758 or #1759 either one would work to get this moving.
Closes: #1057