Makefile: 'make flake8' tests the entire codebase #879
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
added 4 commits
August 30, 2018 10:53
It is safer and just as fast to have flake8 test the entire codebase.
|
Good idea. For the Those are variables provided by the pynsist installer framework, so they're invisible. |
davvid
added a commit
to davvid/git-cola
that referenced
this pull request
Aug 31, 2018
* cclauss/patch-1: Fix whitespace in extras/__init__.py to placate flake8 Fix multiple imports on one line in share/doc/git-cola/conf.py $(FLAKE8) $(FLAKE8_FLAGS) --exclude=./qtpy . Makefile: 'make flake8' tests the entire codebase Signed-off-by: David Aguilar <davvid@gmail.com>
davvid
added a commit
to davvid/git-cola
that referenced
this pull request
Aug 31, 2018
Closes git-cola#882 Related-to: git-cola#879 Reported-by: @cclauss Signed-off-by: David Aguilar <davvid@gmail.com>
davvid
added a commit
to davvid/git-cola
that referenced
this pull request
Aug 31, 2018
The user might have virtualenvs and/or python installations (via contrib/build-git-cola.sh) in their worktree, so invoking flake8 with '.' is too wide a net and can be very slow. We expect virtualenvs and tox environments to exist, per the .gitignore rules, but the flake8 invocation would pick them up. It's also unclear whether the `--exclude` flag replaces, or augments the built-in exclusion list. From the looks of it, the flag full-on replaces the built-in patterns, which means that we would also need to list `.git`, `__pycache__`, and other possible directories in the exclusion list. Maintaining an exclusion list in a world of virtualenvs is more effort than using $(ALL_PYTHON_DIRS) in the target. In light of this, it's better to be consistent with the rest of the Makefile targets. Use $(ALL_PYTHON_DIRS) and add `contrib/win32` to the list of checked python directories so that the pynsist preamble is picked up by pylint and flake8. Related-to: git-cola#879 Reverts: af26369 Signed-off-by: David Aguilar <davvid@gmail.com>
|
Merged, but I ended up reverting the part the makes it use |
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.
It is safer and just as fast to have flake8 test the entire codebase.
I do not know of a safe way to fix the remaining undefined name pkgdir:
https://github.com/git-cola/git-cola/blob/master/contrib/win32/pynsist-preamble.py#L9