-
Notifications
You must be signed in to change notification settings - Fork 71
Dmi resource sane_product to avoid in duplication (bugfix)
#2046
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2046 +/- ##
==========================================
+ Coverage 51.55% 51.94% +0.39%
==========================================
Files 385 388 +3
Lines 41439 41766 +327
Branches 7697 7754 +57
==========================================
+ Hits 21363 21697 +334
+ Misses 19315 19300 -15
- Partials 761 769 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fernando79513
left a comment
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.
Thanks for the change.
I checked in my laptop and seems to be detected correctly.
The change is simple enough that I don't think further on testing on real devices is required.
Nevertheless, I think you should add some unit tests for this parsing using the previous fields:
'Notebook','Laptop','Portable','All In One','All-In-One','AIO','Convertible', 'Tablet', 'Detachable' -> "portable"
'Desktop','Low Profile Desktop','Tower','Mini Tower','Space-saving', 'Mini PC' -> not-portable
fernando79513
left a comment
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.
LGTM +1!
* Add sane_product key * Move all hardcoded consts to new sane_product * Test sane_product translator * Also test the unknown
* Add sane_product key * Move all hardcoded consts to new sane_product * Test sane_product translator * Also test the unknown
* Add sane_product key * Move all hardcoded consts to new sane_product * Test sane_product translator * Also test the unknown
Description
In many resource expression there is a duplicated array that is periodically updated to include new items as laptop like or dektop like. This adds a new key to the resource and uses that instead so that updates are way easier to maintain.
Resolved issues
N/A
Documentation
Added an explanation docstring to say why this is done
Tests
N/A
Please while reviewing this be very carefult that I actually succesfully moved desktops to non-portal and laptops to portable