-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add new filtering options, continue deprecations #2483
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2483 +/- ##
==========================================
+ Coverage 84.52% 93.54% +9.01%
==========================================
Files 622 11 -611
Lines 35060 558 -34502
Branches 4270 66 -4204
==========================================
- Hits 29636 522 -29114
+ Misses 4137 28 -4109
+ Partials 1287 8 -1279 |
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.
Great job ! Just a few minor comments
valid_patterns.append(pattern) | ||
|
||
if valid_patterns: | ||
return re.compile('|'.join(valid_patterns), casing) |
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.
I think we need to make sure that valid_patterns
are enclosed in parenthesis before joining them. Otherwise a pattern containing |
could break the matching no ?
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.
I don't think so as a|b|c
== (a|b)|c
, right?
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.
Yeah I thought I had a example but was mistaken, you're probably right
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.
Just a few more function names comments and it will be 👌
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, looks good now 💯
What does this PR do?
Adds new partition filtering options
file_system_whitelist
file_system_blacklist
device_whitelist
device_blacklist
mount_point_whitelist
mount_point_blacklist
And began deprecation of
excluded_filesystems
excluded_disks
excluded_disk_re
excluded_mountpoint_re
Motivation
Additional Notes
Also converted config to new style