Birds 269 new substitution parameter string to float#71
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new formatter to convert strings to floating-point numbers with 2 decimal places precision.
Changes:
- Added a new
to_fformatter class that converts strings to floats - Comprehensive test coverage for the new formatter including edge cases
- Updated version to 3.1.0.wb.1 and documented the change in CHANGELOG
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/parameter_substitution/formatters/to_f.rb | Implements the new ToF formatter class with string-to-float conversion logic |
| spec/lib/parameter_substitution/formatters/to_f_spec.rb | Provides comprehensive test coverage for the ToF formatter including valid inputs, nil/empty strings, and invalid inputs |
| lib/parameter_substitution/version.rb | Updates the version number to reflect the new feature |
| CHANGELOG.md | Documents the addition of the new formatter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CHANGELOG.md
Outdated
|
|
||
| ## [3.1.0.wb.1] - Not Released | ||
| ### Added | ||
| - Added `string_to_float` formatter, which converts a string to a float value. |
There was a problem hiding this comment.
The formatter key is 'to_f' but the changelog refers to it as 'string_to_float'. These should be consistent - either update the changelog to reference 'to_f' or update the key in the code to match 'string_to_float'.
| - Added `string_to_float` formatter, which converts a string to a float value. | |
| - Added `to_f` formatter, which converts a string to a float value. |
| expect(@format_class.format("")).to eq(nil) | ||
| expect(@format_class.format(" ")).to eq(nil) | ||
| end | ||
|
|
There was a problem hiding this comment.
What do you think about treating non-numeric inputs as nil vs 0.0? My thinking is it could make it potentially easier to debug and work-around (using the if_nil functionality). If we use 0.0 it could be ambiguous if the value is actually zero or if we encountered an unexpected/misconfigured value
There was a problem hiding this comment.
We'll want to update this to the final release version prior to merging
hansondr
left a comment
hansondr
left a comment
There was a problem hiding this comment.
LGTM, just awaiting final version + changelog update but marking this as approved to not hold up merging 👍
2 participants
Jira
Description