-
Notifications
You must be signed in to change notification settings - Fork 0
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 functionality to skip files during isort #196
Conversation
ac1eb68
to
35885f9
Compare
Coverage reportThe coverage rate went from
Diff Coverage details (click to unfold)nengo_bones/version.py
|
@@ -23,6 +23,12 @@ force_exclude = ''' | |||
[tool.isort] | |||
profile = "black" | |||
src_paths = ["{{ pkg_name }}"] | |||
{% if isort_exclude %} | |||
skip_glob = [ |
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 should use extend_skip_glob
here so that it appends to the default exclusions rather than overwriting them.
While I was looking at the docs I also noticed the skip_gitignore
option. I think we should probably enable that. That's the default behaviour for black
, so that makes them consistent in that regard, and likely enabling that option would eliminate the need to use isort_exclude
except in special circumstances.
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.
Ah okay nice, I didn't see the extend_skip_glob
option.
I was initially trying to use the skip_gitignore
flag but I couldn't get it to find the .gitignore
file in the project directory. Although, it's possible that the regular expression that was in the .gitignore
for my use case wasn't compatible with what isort
expects. I agree that it would be much cleaner to use the .gitignore
rather than customizing the exclusions (I imagine all of the relevant exclusions would be found in the .gitignore
anyways).
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've pushed the fixup for adding the skip_gitignore
flag but I'm still in the midst of seeing if I can get it to work in my use case. It works with extend_skip_glob
at the moment, but ceases to work if I remove that from the config.
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.
Ah the problem is that skip_gitignore
traverses and checks each file that satisfies the patterns in .gitignore
and THEN skips them. Whereas, extend_skip_glob
just bakes those patterns into the initial traversal. See this issue for reference PyCQA/isort#1912 (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.
Ah that's annoying. Hopefully they fix that at some point, but won't hurt to have the isort_exclude
option anyway.
6608008
to
49eb386
Compare
49eb386
to
d51fa91
Compare
Motivation and context:
When running the
static
script,isort
can run for an extremely long period of time if there are directories inside the project with large numbers of non-Python files (e.g. text files, binaries, etc.). Adding the ability to skip these directories/files with patterns alleviates this problem.How has this been tested?
Added a new test and ran the entire suite locally.
How long should this take to review?
Types of changes:
Checklist: