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

Move more setup stuff to pyproject.toml #542

Merged
merged 1 commit into from
Mar 5, 2023
Merged

Conversation

claudep
Copy link
Contributor

@claudep claudep commented Mar 4, 2023

No description provided.

@codecov
Copy link

codecov bot commented Mar 4, 2023

Codecov Report

Merging #542 (e58df4e) into master (bff435d) will not change coverage.
The diff coverage is n/a.

❗ Current head e58df4e differs from pull request most recent head 09745ad. Consider uploading reports for the commit 09745ad to get more accurate results

@@           Coverage Diff           @@
##           master     #542   +/-   ##
=======================================
  Coverage   91.28%   91.28%           
=======================================
  Files          28       28           
  Lines        2719     2719           
=======================================
  Hits         2482     2482           
  Misses        237      237           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Nice! I think we can now delete setup.py completely?

We should also change this in the three workflows:

-cache-dependency-path: "setup.py"
+cache-dependency-path: "pyproject.toml"

tox.ini Outdated Show resolved Hide resolved
@claudep
Copy link
Contributor Author

claudep commented Mar 4, 2023

Nice! I think we can now delete setup.py completely?

I read that it might be useful for some older environments, but if you think it's not needed, we can remove it too (and see if we get complaints 😉).

@hugovk
Copy link
Member

hugovk commented Mar 4, 2023

Let's try without the stub setup.py.

https://setuptools.pypa.io/en/latest/userguide/quickstart.html#basic-use says (emphasis added):

We also recommend users to expose as much as possible configuration in a more declarative way via the pyproject.toml or setup.cfg, and keep the setup.py minimal with only the dynamic parts (or even omit it completely if applicable).

And https://setuptools.pypa.io/en/latest/userguide/quickstart.html#development-mode says:

Prior to pip v21.1, a setup.py script was required to be compatible with development mode. With late versions of pip, projects without setup.py may be installed in this mode.

If you have a version of pip older than v21.1 or is using a different packaging-related tool that does not support PEP 660, you might need to keep a setup.py file in file in your repository if you want to use editable installs.

If anyone needs editable installs, I think it's fair enough to require modern pip (= less than two years old).

We can always add the stub back if it's really needed.

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@claudep
Copy link
Contributor Author

claudep commented Mar 5, 2023

OK, fine. setup.py removed!

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

🚀

@hugovk hugovk merged commit e277fc0 into jazzband:master Mar 5, 2023
@claudep claudep deleted the pyproject branch March 5, 2023 21:35
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.

2 participants