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

Add functions for each Value case #9736

Merged
merged 5 commits into from Jul 21, 2023
Merged

Conversation

IanManske
Copy link
Member

Description

This PR ensures functions exist to extract and create each and every Value case. It also renames Value::boolean to Value::bool to match Value::test_bool, Value::as_bool, and Value::Bool. Similarly, Value::as_integer was renamed to Value::as_int to be consistent with Value::int, Value::test_int, and Value::Int. These two renames can be undone if necessary.

User-Facing Changes

No user facing changes, but two public functions were renamed which may affect downstream dependents.

@fdncred
Copy link
Collaborator

fdncred commented Jul 21, 2023

+1 to these changes. I think it cleans up our code a bit. Thanks!

@fdncred fdncred added the pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes label Jul 21, 2023
Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

this looks great ✨

i'm not sure i understand the user change which appears to be a breaking change too 😕

Copy link
Collaborator

@fdncred fdncred left a comment

Choose a reason for hiding this comment

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

I'm ready

@fdncred
Copy link
Collaborator

fdncred commented Jul 21, 2023

i labeled it a breaking change because the api is changing.

@fdncred fdncred merged commit 7e1b922 into nushell:main Jul 21, 2023
19 checks passed
@amtoine
Copy link
Member

amtoine commented Jul 21, 2023

i labeled it a breaking change because the api is changing.

ooh ok, the API of the crates, makes sense 👌

@IanManske IanManske deleted the value-functions branch July 21, 2023 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:breaking-change This PR implies a change affecting users and has to be noted in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants