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.

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

fix(convert): Revert PR #1108 "return error when conversion exceeds f64 bounds" #1179

Merged
merged 2 commits into from
Dec 6, 2024
Merged

fix(convert): Revert PR #1108 "return error when conversion exceeds f64 bounds" #1179

merged 2 commits into from
Dec 6, 2024

Conversation

pront
Copy link
Member

@pront pront commented Dec 6, 2024 •
edited
Loading

Summary

Reverts #1108
Due to vectordotdev/vector#21970. We don't have numbers to prove this, but parsing "0" might be a common operation. And we don't want parse_float and to_float to diverge.

To err on the side of caution let's revert this change. We will work to provide safe arithemtic operations in the future (inspired by Rust APIs).

@pront pront requested review from bfung and jszwedko December 6, 2024 19:04
@pront pront added meta: regression This is a regression from a previous version vrl: stdlib Changes to the standard library labels Dec 6, 2024
@pront
Copy link
Member Author

pront commented Dec 6, 2024

Do we usually add a fix changelog in these cases? cc @jszwedko

@pront pront mentioned this pull request Dec 6, 2024
Merged
11 tasks
@pront pront changed the title Revert "fix(convert): return error when conversion exceeds f64 bounds" fix(convert): Revert PR #1108 "return error when conversion exceeds f64 bounds" Dec 6, 2024
@jszwedko
Copy link
Member

jszwedko commented Dec 6, 2024

Do we usually add a fix changelog in these cases? cc @jszwedko

Yeah, I think a fix changelog is right here.

@pront pront added this pull request to the merge queue Dec 6, 2024
Merged via the queue into main with commit f72967c Dec 6, 2024
14 checks passed
@pront pront deleted the revert-1108-main branch December 6, 2024 21:22
pront added a commit that referenced this pull request Dec 9, 2024
@pront
fix(convert): Revert PR #1108 "return error when conversion exceeds f…
7772d59 
…64 bounds" (#1179)

* Revert "fix(convert): return error when conversion exceeds f64 bounds (#1108)"

This reverts commit 2ee31a4.

* add changelog
@PetrHeinz PetrHeinz mentioned this pull request Dec 10, 2024
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jszwedko jszwedko jszwedko approved these changes

@bfung bfung Awaiting requested review from bfung

Assignees
No one assigned
Labels
meta: regression This is a regression from a previous version vrl: stdlib Changes to the standard library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pront @jszwedko