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

Use a pyproject.toml #4

Merged
merged 6 commits into from
Oct 21, 2024
Merged

Use a pyproject.toml #4

merged 6 commits into from
Oct 21, 2024

Conversation

Baduit
Copy link
Contributor

@Baduit Baduit commented Oct 21, 2024

Using a pyproject.toml is now the way to go. The setup.py can still be used then the build backend is setuptools, especially for dynamic fields (in this case the C Extension a different flags depending on the platform)

It make it easier to install directly with a `pip install git+https://github.com/Baduit/cydoomgenericr`
The setup.py now only contains stuff related to the cython extension.
Copy link
Owner

@wojciech-graj wojciech-graj left a comment

Choose a reason for hiding this comment

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

Thanks a bunch for this! I started converting my python projects to use pyproject.toml recently, but I hadn't gotten around to cydoomgeneric yet, so this is a very welcome change.
Before I merge, I'd appreciate if you could fix the workflow file per the comment I left, and add a line under line 2 in setup.py with the contents Copyright(C) 2024 <YOUR NAME>

@@ -24,12 +24,15 @@ jobs:
uses: actions/setup-python@v3
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
- name: Install cydoomgeneric
working-directory: ${{github.workspace}}/cydoomgeneric
Copy link
Owner

Choose a reason for hiding this comment

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

Either working-directory: ${{github.workspace}}, or this line might be completely redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed , there is no need to set the working-directory as the setup.py and pyproject.toml are at the top level

There is no need to install the requirements manually, with the pyproject.toml it is done automatically

Use the modern replacement for buildind wheels (instead of the python setup.py bdist_wheel)
@wojciech-graj wojciech-graj merged commit 735b4e4 into wojciech-graj:master Oct 21, 2024
2 checks passed
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