-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Improve packaging [PEP 517 + 621] #7076
Conversation
* PEP 517 * Include readme in sdist to be able to build wheel from it * Update release ci job to use build
This comment has been minimized.
This comment has been minimized.
Pull Request Test Coverage Report for Build 2576530244
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you ! I wanted to wait for pep660 but what's left of the setup.cfg is really manageable in the end. This would close #4193
Is there no change in the contributor doc at all ? |
I first checked with just ´[build-system]
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not remove setup.py
just yet:
dependabot/dependabot-core#4483
This is quite an annoying bug.
This reverts commit 7798d91.
a6590db
to
ff074bf
Compare
Update Everything took a bit longer then I expected, but it's ready now.
Turns out if you remove the
Thanks @DanielNoord! Didn't know about that. I've restored |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect ! We've come a long way from the 2000's packaging !
@@ -93,6 +28,11 @@ known_third_party = platformdirs, astroid, sphinx, isort, pytest, mccabe, six, t | |||
skip_glob = tests/functional/**,tests/input/**,tests/extensions/data/**,tests/regrtest_data/**,tests/data/**,astroid/**,venv/** | |||
src_paths = pylint | |||
|
|||
[flake8] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this go in .flake8
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do it the other way round and move the whole flake8 config into setup.cfg
. In the past, we have also removed .isort
. At some point we can move most / all of these configs into pyproject.toml
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong agree. I don't think .flake8
support pyproject.toml right now though . But there's other tool we could move already, tox and black probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong agree. I don't think
.flake8
support pyproject.toml right now though . But there's other tool we could move already, tox and black probably.
I would recommend to keep tox
(and pylint
) separate. Both have quite complex configurations. It's much easier to have them in their individual files. We could however move isort
, pytest
, and mypy
.
This comment has been minimized.
This comment has been minimized.
@DanielNoord dependabot/dependabot-core#4483 doesn't seem to be an issue anymore. I just checked one of my repos and dependabot runs fine even without a |
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit d380aca |
Are you sure? These are the logs/statuses for |
Yeah! This is from My suggestion would be to just try it without |
Let's do that. I think @Pierre-Sassoulas csn report whether the check is running successfully afterwards! |
Seems like you were right @DanielNoord 😞 Triggered the run manually just now. That just raises the question: Why does it work in my repo https://github.com/cdce8p/python-typing-update? |
@cdce8p I have no idea really... The annoying thing is that in de dependabot issue there is already a solution. But my complete lack of ruby experience makes it impossible to contribute any test so I hesitated to create a PR... |
Followup PR: #7082 |
Followup PR: #7249. |
Description
python -m build
instead of deprecatedpython setup.py sdist bdist_wheel
setup.cfg
topyproject.toml
PEP 621MANIFEST.in
-> entries haven't been used.AddREADME.rst
toMANIFEST.in
. Required to be able to build thewheel
fromsdist
and still include the readme text in the project metadata.setup.py
. With recent pip versions (>= 21.0
), no longer required for editable installs.setup.cfg
is enough.Metadata diff
Closes #4193