Skip to content
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

pyflakes is not being registered into flake8 #6999

Closed
frewsxcv opened this issue Aug 5, 2015 · 2 comments
Closed

pyflakes is not being registered into flake8 #6999

frewsxcv opened this issue Aug 5, 2015 · 2 comments
Assignees

Comments

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Aug 5, 2015

Steps to reproduce:

  1. Add unused Python import
  2. Make sure flake8 and pyflakes are not installed locally (to keep things simple)
  3. Run ./mach test-tidy and notice that the unused import does not cause a warning

A little background: Flake8 does not require pyflakes to be installed, and can be run without it. If pyflakes is installed, flake8 will run the checks it provides (like finding unused imports).

It's not working because flake8 utilizes this to register pyflakes into its system. Since setup.py doesn't get run on flake8 (since we're doing the hacky method of just adding .whl's to sys.path), pyflakes never gets registered into flake8.

The former issues is much more trivial than the latter. I looked into this briefly and couldn't think of an easy solution. Considering the benefits aren't that significant, I'm going to punt on this for now. I'll assign it to myself for now and look into it later. Maybe I'll get proper Python dependency management set up in Servo some day...

@frewsxcv frewsxcv self-assigned this Aug 5, 2015
@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Aug 5, 2015

Maybe the easiest solution would be to run pyflakes manually?

@frewsxcv
Copy link
Member Author

@frewsxcv frewsxcv commented Aug 5, 2015

That is possibly an option. If we decide to go that route, we might be able to scrap flake8 altogether and just run pyflakes and pep8 separately.

I don't think automating setting a virtual environment with proper installation of Python dependencies should be too difficult. I think that would be the proper solution here, so I'd like to explore that before resorting to expanding the current hacks.

frewsxcv added a commit to frewsxcv/servo that referenced this issue Aug 8, 2015
Prior to this commit:

* Our Python dependency story was a bit of a mess. We had complete
 Python packages (wheels and directories) living in-tree, despite
 not having any changes from upstream. This is particularly bad because
 `setup.py` never gets run on these packages which could (sometimes
 silently) unintended breakage.
* Python virtual environments (virtualenv) were only utilized for
 testing web-platform tests

After this commit:

* A single virtualenv (`python/_virtualenv`) is activated upon *every*
 call to mach
* A requirements file (`python/requirements.txt`) is added to describe
 the dependencies needed by Python modules in `python/`. The child
 commit immediately following this will remove all the dependencies
 no longer needed in-tree (for the sake of keeping this commit
 readable).

Relevant to servo#861

Fixes servo#6999
@frewsxcv frewsxcv closed this in 33f7831 Aug 9, 2015
josiahdaniels added a commit to josiahdaniels/servo that referenced this issue Sep 28, 2015
Prior to this commit:

* Our Python dependency story was a bit of a mess. We had complete
 Python packages (wheels and directories) living in-tree, despite
 not having any changes from upstream. This is particularly bad because
 `setup.py` never gets run on these packages which could (sometimes
 silently) unintended breakage.
* Python virtual environments (virtualenv) were only utilized for
 testing web-platform tests

After this commit:

* A single virtualenv (`python/_virtualenv`) is activated upon *every*
 call to mach
* A requirements file (`python/requirements.txt`) is added to describe
 the dependencies needed by Python modules in `python/`. The child
 commit immediately following this will remove all the dependencies
 no longer needed in-tree (for the sake of keeping this commit
 readable).

Relevant to servo#861

Fixes servo#6999
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.