use shell globs on file name for include/exclude patterns #137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
beniwohli
force-pushed
the
include-patterns-glob
branch
3 times, most recently
from
7fec857
to
da11fd9
Compare
graphaelli
approved these changes
Jan 19, 2018
| if not tmp_client: | ||
| _client = (client, instance) | ||
| return instance | ||
| return _client[1] | ||
|
|
||
|
|
||
| def get_client_default_config(): | ||
| return dict( | ||
| include_paths=_get_installed_apps_paths(), |
There was a problem hiding this comment.
looks like _get_installed_apps_paths was only called from here, could remove that function too
|
I'm unclear on what will happen on windows but noticed your TODO, not sure how you normally handle that. |
|
Regarding Windows, I did a quick test with Python 3.6 So ( I'll do some more tests with Python 2 and pypy and then add Windows-specific patterns. |
beniwohli
force-pushed
the
include-patterns-glob
branch
2 times, most recently
from
6d1276c
to
d8071b0
Compare
The previously used module path was very brittle. Especially in the case of Django, where the default `include_path` was the set of `INSTALLED_APPS`, this would have a lot of false positives, namely all 3rd party Django apps. Using the `lib/python` exclude patterns should work for all cases that use virtualenvs or the system python.
beniwohli
force-pushed
the
include-patterns-glob
branch
from
d8071b0
to
52265a4
Compare
beniwohli
added a commit
that referenced
this pull request
Jan 24, 2018
The previously used module path was very brittle. Especially in the case of Django, where the default `include_path` was the set of `INSTALLED_APPS`, this would have a lot of false positives, namely all 3rd party Django apps. Using the `lib/python` exclude patterns should work for all cases that use virtualenvs or the system python. closes #137
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previously used module path was very brittle. Especially in the
case of Django, where the default
include_pathwas the set ofINSTALLED_APPS, this would have a lot of false positives, namelyall 3rd party Django apps.
Using the
lib/pythonexclude patterns should work for all cases thatuse virtualenvs or the system python.