-
Notifications
You must be signed in to change notification settings - Fork 15
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
Properly package the canonicaljson module #52
Conversation
include LICENSE | ||
include tox.ini | ||
include 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.
These both seem like good things to include in the sdist. Are they included automatically now?
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.
As per #synapse-dev, it looks like python -m build
always includes all files from the repository by default when run on my machine, not sure why.
According to https://packaging.python.org/en/latest/guides/using-manifest-in/, pyproject.toml
is automatically included now. I've also added the license file to license_file
in setup.cfg
since it seems like that will always cause it to be included.
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've now replaced the content of MANIFEST.in
with the suggestion from check-manifest
. The only exception to this is that I've added a prune .github
to remove GitHub codeowner and workflow files, which felt to me like they shouldn't live in the sdist.
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.
Thanks, SGTM. Are you okay to make a (patch) release with these changes?
Yep that was my plan once this is merged (I need the changes released for something else anyway). |
#52 introduced a build dependency on `setuptools_scm`. According to https://github.com/pypa/setuptools_scm, usage of `setuptools_scm` involves either a `[tool.setuptools_scm]` section in `pyproject.toml`, or `use_scm_version=True` in `setup.py`. We do neither of those things, so this build requirement ought to be safe to remove. Signed-off-by: Sean Quah <[email protected]>
#52 introduced a build dependency on `setuptools_scm`. According to https://github.com/pypa/setuptools_scm, usage of `setuptools_scm` involves either a `[tool.setuptools_scm]` section in `pyproject.toml`, or `use_scm_version=True` in `setup.py`. We do neither of those things, so this build requirement ought to be safe to remove. Signed-off-by: Sean Quah <[email protected]>
since matrix-org/python-canonicaljson#52 which is included in the lockfile version we use for type checking.
* canonicaljson has stubs now since matrix-org/python-canonicaljson#52 which is included in the lockfile version we use for type checking. * Changelog
Package canonicaljson as a proper module rather than a single-file module. The main motivation is to expose the
py.typed
marker file so other projects can use the typing annotations.I've decided to go with an
src/
layout for the module's source rather than the solution proposed in #50 (comment) to avoid a repeat of matrix-org/matrix-python-common#23.Fixes #50