-
-
Notifications
You must be signed in to change notification settings - Fork 67
Migrate build and setup from setup.py to pyproject.toml #52
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
Conversation
e6bf2e0
to
e23e955
Compare
@@ -1 +1,3 @@ | |||
__version__ = '0.0.5' | |||
import pkg_resources |
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.
Should we use the newer importlib.metadata instead?
@@ -0,0 +1,71 @@ | |||
[build-system] | |||
requires = ["setuptools", "setuptools-scm"] |
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.
I wouldn't choose setuptools as the build backend as it has to support many legacy workflows that prevent innovations in other tools. I would propose hatchling.build
as the official tutorial does https://packaging.python.org/en/latest/tutorials/packaging-projects/#creating-pyproject-toml (although implicitly as listing it as the first choice)
|
||
[project] | ||
name = "python-multipart" | ||
version = "0.0.5" |
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.
Should we bump to 0.0.6?
'Topic :: Software Development :: Libraries :: Python Modules' | ||
] | ||
dependencies = [ | ||
"atomicwrites==1.2.1", |
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.
Dependencies should not be hard pinned here as it forces ALL users of this lib to use SPECIFICALLY this version. More relaxed requirements as atomicwrites>=1.0.0
are prefered. The minimum version that works, the better, that allows users more freedom to pin in their apps without a dependency (that's even transitive in the case of fastapi users) forcing you to certain version that might have a security vuln for example
"Homepage" = 'https://github.com/andrew-d/python-multipart' | ||
|
||
[project.optional-dependencies] | ||
dev = [ |
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.
If we were to pin, here would be better as it will only be done if we're setting up a dev enviornment, not a library usage environment
Thank you for your work Axel! Left my review, let me know if i wasn't clear in any of my suggestions |
Thanks for the review Nahuel, I'll work on it tomorrow |
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 we try to match the build setup that starlette
has?
Hi, let me know if you need some help. If you don't have time i can update the or too with @Kludex suggestion |
Hi @Ambro17 : It's been pretty crazy at work lately. If you want to implement changes, please feel free to do so |
@Ambro17 do you take care of this or should I? |
Ah, this was already done. |
As discussed in #37
Here is the first part towards automating the release process. You can now run the following code to build the package :
This will generate both
.whl
and.tar.gz
files, last step will be to publish them to pypi using twine.What is in this PR ?
requirements.txt
&setup.py
topyproject.toml
__version__
from package topyproject.toml