-
Notifications
You must be signed in to change notification settings - Fork 39
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
fix: handle empty array as nil when filling record defaults #345
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
==========================================
+ Coverage 52.77% 52.84% +0.06%
==========================================
Files 69 69
Lines 5095 5102 +7
==========================================
+ Hits 2689 2696 +7
Misses 1832 1832
Partials 574 574
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 things I'd like to raise here:
- Add a reference of koko case in the code comment so that we know which issue this is related to
- This definitely deserves a changelog entry
- A test case should be added. There already exists
Test_fillConfigRecord
so it should be as easy as adding another table driver test case there
177c7d4
to
aa7fca2
Compare
aa7fca2
to
6286cdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nits, I applied those right away to speed up the delivery of this patch.
Thanks for adding changelog and tests! 🙇
To avoid panic errors, in case of a wrong
[]
being set in the config for a record field, let's treat it as a nil and create the proper object map.Can be reproduced with a deck sync: