search

aconrad / pycobertura

A code coverage diff tool for Cobertura reports
MIT License
114 stars 39 forks source link

move test requirements into setup.cfg #141

Closed gro1m closed 1 year ago

gro1m commented 2 years ago

Move tests into setup.cfg. Would also allow to pin version of pytest.

gro1m commented 2 years ago

@aconrad Do not merge before #140 is merged and this needs some merge conflict resolution, especially, the pytest-cov package must disappear (because it sets the source that caused as so much trouble).

aconrad commented 2 years ago

Do we still need/want this PR @gro1m?

aconrad commented 2 years ago

Isn't the industry standard to use test-requirements.txt? What motivates this change?

gro1m commented 2 years ago

See the discussion here (there are preferences for either, but maybe it can help you form an opinion on this): https://discuss.python.org/t/is-it-preferable-to-add-test-dependensies-as-an-extra-require-instead-of-tox-ini/4984.

aconrad commented 2 years ago

Typing pip install -e .[test] feels a little more cryptic to me, but it's likely just about habit. But I haven't worked professionally with Python for a couple of years and if that's how people like to do it nowadays, then fine by me.

aconrad commented 2 years ago

The PR description says that it would "allow to pin version of pytest". Why was that not already the case with test-requirements.txt?

gro1m commented 2 years ago

The PR description says that it would "allow to pin version of pytest". Why was that not already the case with test-requirements.txt?

I am not sure why I put that. At the time, I thought that would be a particular advantage, but it is also possible with test-requirements.txt, as I did in my last PR.

gro1m commented 2 years ago

But I think the discussion is more if you want to keep the test requirements external to the package or not. And what is nicer with the setup.cfg is that you do not explicitly specify the current package with a . (i.e. pycobertura itself)