-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Support globbing pattern for input specification #8312
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
Support globbing pattern for input specification #8312
Conversation
de7f3ee
to
7061f87
Compare
7061f87
to
357557a
Compare
@Pierre-Sassoulas @DudeNr33 do we want to add this to 2.17 as well? The CI misbehaved here as well |
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 closed/opened to launch the CI again, feel free to do that or to rebase on upstream/main and push force if you need to :)
doc/user_guide/usage/run.rst
Outdated
With globbing pattern | ||
--------------------- | ||
|
||
It is also possible to specify both directories and files using globbing patterns:: | ||
|
||
pylint [options] packages/*/src | ||
|
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.
Imo this should be in the On module packages or directories
paragraph
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.
Yet it's also applicable to On files
- it can't be in both paragraphs at the same time
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.
IMO renaming the headline to Globbing support
would be good, as it makes it a bit clearer that this section just informs you about the possibility to use glob patterns.
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.
Globbing permits to handle file/module/package, the distinction could be removed entirely maybe ?
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.
IMHO, that section of docs should be refactored, yet I don't have enough knowledge to do that confidently
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #8312 +/- ##
==========================================
+ Coverage 95.46% 95.56% +0.09%
==========================================
Files 177 178 +1
Lines 18704 18765 +61
==========================================
+ Hits 17856 17932 +76
+ Misses 848 833 -15
|
This comment has been minimized.
This comment has been minimized.
To fix the spelling check you can add |
Co-authored-by: Pierre Sassoulas <[email protected]>
This comment has been minimized.
This comment has been minimized.
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.
Only two minor remarks from my side. Logic looks good to me.
doc/user_guide/usage/run.rst
Outdated
With globbing pattern | ||
--------------------- | ||
|
||
It is also possible to specify both directories and files using globbing patterns:: | ||
|
||
pylint [options] packages/*/src | ||
|
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.
IMO renaming the headline to Globbing support
would be good, as it makes it a bit clearer that this section just informs you about the possibility to use glob patterns.
This comment has been minimized.
This comment has been minimized.
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit a8865f4 |
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!
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, it makes 2.17.0 consistent (all the globbing added) and we can refactor the doc in #7496.
Type of Changes
Description
Closes #8310