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

DEV: Delete tox.ini #2713

Merged
merged 4 commits into from
Jun 20, 2024
Merged

DEV: Delete tox.ini #2713

merged 4 commits into from
Jun 20, 2024

Conversation

j-t-1
Copy link
Contributor

@j-t-1 j-t-1 commented Jun 11, 2024

Add Python 3.11, 3.12, remove 3.6

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.14%. Comparing base (a195cb3) to head (ab7c6ff).

Additional details and impacted files
@@           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           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stefan6419846
Copy link
Collaborator

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
Copy link
Collaborator

is this file used for coverage analysis ?

@stefan6419846
Copy link
Collaborator

AFAIK no, although it seems like we have a section about testing with tox in our testing docs. The coverage configuration is

pypdf/pyproject.toml

Lines 85 to 112 in 683aeca

[tool.coverage.run]
source = ["pypdf"]
branch = true
[tool.coverage.report]
# Regexes for lines to exclude from consideration
exclude_lines = [
# Have to re-enable the standard pragma
"pragma: no cover",
"@overload",
"deprecated",
# Don't complain about type-checking code not being hit by unit tests
"if TYPE_CHECKING",
# Don't complain about missing debug-only code:
"def __repr__",
"def __str__",
"if self\\.debug",
# Don't complain if tests don't hit defensive assertion code:
"raise AssertionError",
"raise NotImplementedError",
# Don't complain if non-runnable code isn't run:
"if 0:",
"if __name__ == .__main__.:",
]

@pubpub-zz
Copy link
Collaborator

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
Copy link
Collaborator

@MartinThoma
+1 ?

@j-t-1 j-t-1 closed this Jun 17, 2024
@j-t-1 j-t-1 deleted the tox branch June 17, 2024 14:53
@j-t-1 j-t-1 restored the tox branch June 17, 2024 14:53
@j-t-1
Copy link
Contributor Author

j-t-1 commented Jun 18, 2024

@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
Copy link
Collaborator

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.

@stefan6419846 stefan6419846 reopened this Jun 18, 2024
@j-t-1
Copy link
Contributor Author

j-t-1 commented Jun 19, 2024

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
Copy link
Collaborator

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

We are not using tox for CI, but GitHub Actions and their workflow matrix, which basically is the same.
@stefan6419846 stefan6419846 changed the title MAINT: Update tox.ini Python versions DEV: Delete tox.ini Jun 20, 2024
@stefan6419846 stefan6419846 merged commit a512408 into py-pdf:main Jun 20, 2024
17 checks passed
@j-t-1 j-t-1 deleted the tox branch June 20, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants