Skip to content

Fix signature of throw #7365

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 31, 2025
Merged

Fix signature of throw #7365

merged 1 commit into from
Mar 31, 2025

Conversation

cknitt
Copy link
Member

@cknitt cknitt commented Mar 25, 2025

No description provided.

@cknitt cknitt requested a review from DZakh March 26, 2025 07:58
tsnobip
tsnobip reviewed Mar 26, 2025
View reviewed changes
Copy link
Member

@tsnobip tsnobip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're still mixing the throwing of rescript and JS exceptions, maybe it's a matter of naming and we should rename Stdlib_Error to Stdlib_JsError and same for exception, because it's a bit confusing right now.

cknitt
cknitt commented Mar 26, 2025
View reviewed changes
@cknitt cknitt marked this pull request as draft March 26, 2025 09:15
@cknitt
Copy link
Member Author

cknitt commented Mar 26, 2025

Setting this to draft mode.

As discussed, we may want to rethink the current situation as a whole, module Error vs. Exn is confusing, and it is unclear at first glance that they are actually referring to JS errors.

@cknitt cknitt changed the title Fix signature of throw and add throwAny Fix signature of throw Mar 30, 2025
@cknitt cknitt marked this pull request as ready for review March 30, 2025 11:50
@cknitt cknitt requested a review from tsnobip March 30, 2025 11:50
@cknitt
Copy link
Member Author

cknitt commented Mar 30, 2025

@tsnobip Reduced the scope of this PR to just fix the incorrect signature of Pervasives.throw.
Any further cleanup or additions (regarding Exn vs. Error vs. JsError) can be done in separate PRs.

tsnobip
tsnobip approved these changes Mar 31, 2025
View reviewed changes
Copy link
Member

@tsnobip tsnobip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@cknitt cknitt merged commit 9942dbd into master Mar 31, 2025
20 checks passed
@cknitt cknitt deleted the fix-throw branch March 31, 2025 07:48
@hackwaly
Copy link

hackwaly commented Apr 1, 2025

Can we have a hotfix release include this?

fhammerschmidt pushed a commit that referenced this pull request Apr 4, 2025
@cknitt @fhammerschmidt
Fix signature of throw (#7365)
3e07d26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsnobip tsnobip tsnobip approved these changes

@nojaf nojaf nojaf approved these changes

@DZakh DZakh Awaiting requested review from DZakh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cknitt @hackwaly @tsnobip @nojaf