-
Notifications
You must be signed in to change notification settings - Fork 77
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 settings to setup.cfg and use setuptools-scm #1065
Conversation
Move config from .flake8, pytest.ini, setup.py, coveragerc, and requirements*.txt to setup.cfg and pyproject.tom. Adopt setuptools-scm instead of versioneer (which will also allow us to delete MANIFEST.in).
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 did my best to review the parts I understood. I also tried to double-check that all the elements from the previous setup scripts found a new home and I didn't notice any discrepancies that made me worry.
I don't have any experience with the github actions configuration and I'm much more familiar with the older setup scripts than the setuptools-scm ways, so hopefully others can review too.
I left a few questions that might be more for my understanding than anything else.
Also: the inline codecov is a bit obnoxious here in the review process. I suspect it's more useful if your repo has high coverage? I hope there's some way to hide these...
I was surprised to see that there was already a Edit: I've just spotted the PR comments it makes, those weren't there when I opened this PR! I'll see if there's a way to turn them off |
versionfile_source = ophyd/_version.py | ||
versionfile_build = ophyd/_version.py | ||
tag_prefix = v | ||
[metadata] |
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 any of this be moved to and used from pyproject.toml
? There is a recommendation to deprecate setup.cfg
:
usage in setup.cfg is considered deprecated, please use pyproject.toml whenever possible
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 think most things can read from pyproject.toml
. tox just embeds the ini under a specific key, which is a bit ugly, but flake8 won't add support for pyproject.toml until pip changes its behaviour on the presence of pyproject.toml. I'll try moving some things and see if it works
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.
Actually, as the [tool.setuptools]
table in pyproject.toml is still in beta I think it's best to leave things in setup.cfg until they have stabilized.
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 is a recommendation to deprecate
setup.cfg
:
It looks like this recommendation is from setuptools-scm, and we already put its configuration in pyproject.toml
Move config from
.flake8
,pytest.ini
,setup.py
,coveragerc
, andrequirements*.txt
tosetup.cfg
and adopt setuptools-scm instead of versioneer (which will also allow us to deleteMANIFEST.in
).Rewrite CI to share steps in less workflows and test the packaging.
@danielballan @tacaswell @mrakitin @klauer @ZLLentz @prjemian @dylanmcreynolds @whs92 @coretl @callumforrester