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

Unify development requirements #923

Merged
merged 1 commit into from
Mar 4, 2020

Conversation

atugushev
Copy link
Member

@atugushev atugushev commented Oct 7, 2019

  • removes dev-requirements.txt in favor of extras_require in setup.py
  • adds testing and coverage extras
  • adds instruction to install pip-tools in development mode with test dependencies in CONTRIBUTING.md
  • updates tox.ini to use extras
  • unpins pytest dependency, since the fix has already released

@atugushev atugushev added the maintenance Related to maintenance processes label Oct 7, 2019
@atugushev
Copy link
Member Author

atugushev commented Oct 7, 2019

FTR, pip==8.1.1 can't handle environment markers, that's why tox installs pytest>=5 which is incompatible with python2.

@atugushev
Copy link
Member Author

atugushev commented Oct 8, 2019

Can we please stop support Python 2 and pip==8.1.1? 😎

@georgek
Copy link
Contributor

georgek commented Oct 9, 2019

Should tox be added to the dev requirements?

@atugushev
Copy link
Member Author

Hello @georgek,

Should tox be added to the dev requirements?

I don't think so. The idea is to have one place for development requirements which can be used by tox or a user itself for a local development and testing pip install -e .[testing].

@georgek
Copy link
Contributor

georgek commented Oct 10, 2019

OK, this actually makes the distinction much more clear as tox is the automation framework and at a higher level than the testing extras. I was confused before about tox not being in dev-requirements.txt, but I suppose a user should know to install toxif he wishes to run the tests using that.

@atugushev
Copy link
Member Author

atugushev commented Oct 10, 2019

@georgek

but I suppose a user should know to install toxif he wishes to run the tests using that.

It would be nice to have this info in CONTRIBUTING.md.

@georgek
Copy link
Contributor

georgek commented Oct 11, 2019

When I run tox -e checkqait fails because it can't find isort, flake8 or bandit. Are these missing dependencies that should be somewhere?

@atugushev
Copy link
Member Author

When I run tox -e checkqait fails because it can't find isort, flake8 or bandit. Are these missing dependencies that should be somewhere?

We use pre-commit now. All the dependencies are in .pre-commit-config.yaml. Could you run tox -re checkqa and show the output?

@georgek
Copy link
Contributor

georgek commented Oct 11, 2019

(pip-tools) gk@gk-pc ~/code/pip-tools (unify-dev-requirements) $ tox -re checkqa
checkqa recreate: /home/gk/code/pip-tools/.tox/checkqa
checkqa installdeps: pre-commit
checkqa installed: aspy.yaml==1.3.0,cfgv==2.0.1,identify==1.4.7,importlib-metadata==0.23,more-itertools==7.2.0,nodeenv==1.3.3,pip-tools==4.1.1.dev26+g49b7e95,pre-commit==1.18.3,PyYAML==5.1.2,six==1.12.0,toml==0.10.0,virtualenv==16.7.5,zipp==0.6.0
checkqa run-test-pre: PYTHONHASHSEED='1448334167'
checkqa run-test-pre: commands[0] | pip --version
pip 19.2.3 from /home/gk/code/pip-tools/.tox/checkqa/lib/python3.7/site-packages/pip (python 3.7)
checkqa run-test: commands[0] | pre-commit run --all-files --show-diff-on-failure
black....................................................................Passed
isort....................................................................Failed
hookid: isort

Traceback (most recent call last):
  File "/home/gk/.cache/pre-commit/repoz0nmzqaw/py_env-python3.7/bin/isort", line 6, in <module>
    from isort.main import main
ModuleNotFoundError: No module named 'isort'
Traceback (most recent call last):
  File "/home/gk/.cache/pre-commit/repoz0nmzqaw/py_env-python3.7/bin/isort", line 6, in <module>
    from isort.main import main
ModuleNotFoundError: No module named 'isort'

flake8...................................................................Failed
hookid: flake8

Traceback (most recent call last):
  File "/home/gk/.cache/pre-commit/repo9z9ih1f6/py_env-python3.7/bin/flake8", line 6, in <module>
    from flake8.main.cli import main
ModuleNotFoundError: No module named 'flake8'

bandit...................................................................Failed
hookid: bandit

Traceback (most recent call last):
  File "/home/gk/.cache/pre-commit/repow88fv026/py_env-python3.7/bin/bandit", line 6, in <module>
    from bandit.cli.main import main
ModuleNotFoundError: No module named 'bandit'
Traceback (most recent call last):
  File "/home/gk/.cache/pre-commit/repow88fv026/py_env-python3.7/bin/bandit", line 6, in <module>
    from bandit.cli.main import main
ModuleNotFoundError: No module named 'bandit'

ERROR: InvocationError for command /home/gk/code/pip-tools/.tox/checkqa/bin/pre-commit run --all-files --show-diff-on-failure (exited with code 1)
________________________________________________ summary _________________________________________________
ERROR:   checkqa: commands failed

@atugushev
Copy link
Member Author

@georgek probably pre-commit clean can help.

@atugushev
Copy link
Member Author

@georgek, could you file a separate issue so that we can discuss it there?

@jayvdb
Copy link
Member

jayvdb commented Oct 16, 2019

small aside, I've been looking at https://github.com/theacodes/nox lately, and am quite impressed with it as a replacement for invoke or stuffing extra non-test commands into tox.ini.

@atugushev
Copy link
Member Author

@jayvdb thanks for reviewing this and your approvement! Unfortunately, it's blocked by Python 2 and pip==8.1.1, so I've started the discussion about deprecation policy in #926.

@atugushev atugushev added the refactor Refactoring code label Oct 17, 2019
@codecov
Copy link

codecov bot commented Oct 20, 2019

Codecov Report

Merging #923 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #923   +/-   ##
=======================================
  Coverage   99.41%   99.41%           
=======================================
  Files          34       34           
  Lines        2411     2411           
  Branches      297      297           
=======================================
  Hits         2397     2397           
  Misses          8        8           
  Partials        6        6

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 888c2a1...556eed8. Read the comment docs.

@atugushev
Copy link
Member Author

@jayvdb I've added a factor specific setting to avoid installing pytest>5 on python2. A hack, but looks good to me.

@atugushev atugushev requested a review from jayvdb October 20, 2019 08:03
@jayvdb
Copy link
Member

jayvdb commented Oct 20, 2019

That approach is ok. The alternatives are messier.

@atugushev atugushev added this to the 5.0.0 milestone Feb 3, 2020
@atugushev
Copy link
Member Author

Let's do it in 5.0.0, where we drop old pip versions.

@atugushev atugushev merged commit 7ef6207 into jazzband:master Mar 4, 2020
@atugushev atugushev deleted the unify-dev-requirements branch March 4, 2020 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Related to maintenance processes refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants