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

Fix pyproject.toml support #16

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ FROM thekevjames/coveralls:latest

COPY src/ /src/

RUN python3 -m pip install "coverage[toml]"

ENTRYPOINT ["/src/entrypoint.py"]
14 changes: 14 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[build-system]
requires = ["setuptools>=42", "wheel"]
build-backend = "setuptools.build_meta"

[tool.coverage.paths]
source = ["src", "*/site-packages"]

[tool.coverage.run]
branch = true
relative_files = true

[tool.coverage.report]
show_missing = true
exclude_lines = ["if __name__ == .__main__.:", "pragma: no-cover", "@abstract", "def __repr__"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line needed for pyproject.toml support? What is it doing? What about the other changes to this file? Is build-system needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build-system section is alternative of setup of setup.py. When there is no setup.py, and when using setuptools for build, it is to be a same configuraiton.

A file pyproject.toml is used for test, not to supporting pyproject.toml in users project.
These lines are adeed for testing purpose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing of the pyproject.toml support? I think the extra changes in this PR were a little confusing otherwise so it was more difficult to review by the maintainer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm well aware of what pyproject.toml is and its purpose. I was only trying to say that it isn't clearly mentioned in any comments or in the PR description what the extra configuration in this pyproject.toml is doing. The coverage configuration is different from the existing configuration in setup.cfg (which is not removed in this PR). You also define a build system where previously there wasn't one so it isn't clear why one is needed.

I'm also a little confused because the tests in push.yml and in make tests don't explicitly handle pyproject.toml so by putting it here in the root of the repository I assume all tests are now using this pyproject.toml. However, that doesn't test the absence of a pyproject.toml (by using the existing setup.cfg). Shouldn't both cases be tested? Perhaps with a subdirectory for different test configuration cases? One with a setup.cfg and one with a pyproject.toml?

1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
black
coveralls
coverage[toml]
flake8
isort
pytest