py-pdf / pypdf

A pure-python PDF library capable of splitting, merging, cropping, and transforming the pages of PDF files
https://pypdf.readthedocs.io/en/latest/
Other
8.12k stars 1.39k forks source link

DEV: Delete tox.ini #2713

Closed j-t-1 closed 3 months ago

j-t-1 commented 3 months ago

Add Python 3.11, 3.12, remove 3.6

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.14%. Comparing base (a195cb3) to head (ab7c6ff). Report is 61 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2713 +/- ## ======================================= Coverage 95.14% 95.14% ======================================= Files 51 51 Lines 8547 8547 Branches 1703 1703 ======================================= Hits 8132 8132 Misses 261 261 Partials 154 154 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

stefan6419846 commented 3 months ago

I am not going to merge this for now to allow discussions on whether we actually still need this file - our CI does not need it.

pubpub-zz commented 3 months ago

is this file used for coverage analysis ?

stefan6419846 commented 3 months ago

AFAIK no, although it seems like we have a section about testing with tox in our testing docs. The coverage configuration is https://github.com/py-pdf/pypdf/blob/683aecac53738e2b71a0e2f0a8a12a112e63aa6d/pyproject.toml#L85-L112

pubpub-zz commented 3 months ago

what about preparing a PR to remove the file, merge and check if all ok and revert if observing side effect ? @MartinThoma any ideas about ?

pubpub-zz commented 3 months ago

@MartinThoma +1 ?

j-t-1 commented 3 months ago

@stefan6419846 I deleted this accidentally, and then restored it.

If you want to get a coverage report that considers the Python version specific code, you can run tox. [testing.md]

It probably is not in the automated testing because it is time consuming to run (although I have not tested this!). We could keep tox.ini and run it before releases.

stefan6419846 commented 3 months ago

We are not using tox for CI, but GitHub Actions and their workflow matrix, which basically is the same. IMHO having the separate CI jobs instead of letting tox iterate over all Python installations drastically improves readability.

tox might be used for local development purposes, but I would argue that most of us do not want to test multiple Python versions locally or do not even have multiple Python versions installed, thus rendering it more or less useless.

j-t-1 commented 3 months ago

We are not using tox for CI, but GitHub Actions and their workflow matrix, which basically is the same. IMHO having the separate CI jobs instead of letting tox iterate over all Python installations drastically improves readability.

tox might be used for local development purposes, but I would argue that most of us do not want to test multiple Python versions locally or do not even have multiple Python versions installed, thus rendering it more or less useless.

Makes sense. If others agree we should delete tox.ini; if we keep it, then this PR would be okay.

pubpub-zz commented 3 months ago

I agree : let's change the PR to delete tox.ini