smarter_csv
smarter_csv copied to clipboard
V1.10.x and stealth add v2 features
Adding Experimental v2 Features 🧪 🌶️
This PR adds the features from the 2.0-develop branch to the main branch, but as hidden / experimental features.
The intent is that users can try out the new behavior and send feedback / contribute PRs.
When option v2_mode: true
is set, the new code paths are used.
Still missing in this PR / TO DO before merging:
- need to update the options processing to have default values in v2_mode that make it behave like v1.x
Thanks @tilo! A couple of questions regarding this (I have not yet looked at the changes):
- Is the intent that if we were using 2.0-pre, that if we switch to this and set the flag, our code should work without any other changes?
- Curious, what's the longer term plan? Are you thinking this this flag is going to be the final form of the 2.0 functionality and it just gets merged into 1.x branch? Or are you planning on making a 2.0 breaking change release that doesn't carry the 1.x baggage at some point?
Thanks @tilo! A couple of questions regarding this (I have not yet looked at the changes):
1. Is the intent that if we were using 2.0-pre, that if we switch to this and set the flag, our code should work without any other changes? 2. Curious, what's the longer term plan? Are you thinking this this flag is going to be the final form of the 2.0 functionality and it just gets merged into 1.x branch? Or are you planning on making a 2.0 breaking change release that doesn't carry the 1.x baggage at some point?
@mscrivo
The 2.0-develop
branch is quite old, and there were many updates on the main branch since then - so the easiest way to consolidate the two versions was to refactor the 1.x code and to pull select changes into the main branch.
This PR is still WIP, so I'm not making promises to be 100% compatible with 2.0-pre, but I'll try.
Because the change from v1.x to v2.x could be disruptive to users, I decided to add both code versions to main for now, and switch them via the v2_mode
option. In v1.x versions this will default to false
, and in future v2.x versions it will default to true
. The idea is that the user can switch over to the new behavior on a more flexible schedule.
Eventually the 1.x code will be deprecated and removed.
with old count_quote_cards:
time: 10.101 ms
allocated memory by class
358020 String
112656 Hash
79656 Array
54096 MatchData
44280 Regexp
8432 File
allocated objects by gem
11030 smarter_csv-1.11.0.pre1
allocated objects by class
8657 String
1789 Array
322 MatchData
171 Hash
90 Regexp
1 File
with new count_quote_chars:
time: 6.043 ms
allocated memory by class
151965 String
112656 Hash
85720 Regexp
82856 Array
54096 MatchData
8432 File
allocated objects by gem
6176 smarter_csv-1.11.0.pre2
allocated objects by class
3643 String
1869 Array
322 MatchData
171 Hash
170 Regexp
1 File
- the new code needs test coverage
- more performance testing
Codecov Report
Attention: Patch coverage is 86.30952%
with 23 lines
in your changes are missing coverage. Please review.
Project coverage is 95.68%. Comparing base (
2f264ca
) to head (1dc31d5
). Report is 5 commits behind head on main.
:exclamation: Current head 1dc31d5 differs from pull request most recent head eb30e70. Consider uploading reports for the commit eb30e70 to get more accurate results
Files | Patch % | Lines |
---|---|---|
lib/smarter_csv/hash_transformations.rb | 58.82% | 21 Missing :warning: |
lib/smarter_csv/options_processing.rb | 94.28% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #267 +/- ##
===========================================
- Coverage 100.00% 95.68% -4.32%
===========================================
Files 11 11
Lines 380 533 +153
===========================================
+ Hits 380 510 +130
- Misses 0 23 +23
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.