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

Make JSContextRef::wrap_rust_value private #490

Merged
merged 1 commit into from
Oct 19, 2023

Conversation

saulecabrera
Copy link
Member

@saulecabrera saulecabrera commented Oct 19, 2023

Description of the change

While investigating #486 I realized that this function is still public, which shouldn't be the case as it's only used for wrapping closure types, which is an internal detail about the crate.

Why am I making this change?

Checklist

  • I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli and javy-core do not require updating CHANGELOG files.
  • I've updated the relevant crate versions if necessary. Versioning policy for library crates
  • I've updated documentation including crate documentation if necessary.

@saulecabrera saulecabrera force-pushed the make-wrap-r-value-private branch from 7a34955 to fdcb9e7 Compare October 19, 2023 14:22
While investigating #486
I realized that this function is still public, which shouldn't be the
case as it's only used for wrapping closure types, which is an internal
detail about the crate.
@saulecabrera saulecabrera force-pushed the make-wrap-r-value-private branch from fdcb9e7 to dba7a16 Compare October 19, 2023 15:41
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Changed
- Make `JSContextRef::wrap_rust_value` private. Similar to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be considered removed? The function is still there, but from the library consumer's point of view, it's a feature that's been removed from the public API. I don't feel strongly about it though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I consider it, but for someone reading the code, or even comparing differences between both versions, I concluded that this is probably the best wording to capture what happened.

@saulecabrera saulecabrera merged commit 18e652f into main Oct 19, 2023
14 checks passed
@saulecabrera saulecabrera deleted the make-wrap-r-value-private branch October 19, 2023 19:05
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.

2 participants