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

Add hosting of built wheel at PyPI - main #297

Open
wants to merge 51 commits into
base: main
Choose a base branch
from
Open

Add hosting of built wheel at PyPI - main #297

wants to merge 51 commits into from

Conversation

kccwing
Copy link
Collaborator

@kccwing kccwing commented Aug 29, 2024

Adding github actions to automatically release new wheel to PyPI on new version tag. Testing added for commits to prepare wheel sans upload. A few changes to setup.py necessitated to enable PyPI. Installation guide and github actions readme updated with (experimental!) documentation.

@kccwing
Copy link
Collaborator Author

kccwing commented Aug 29, 2024

by the way, when we eventually do merge this can we squash and merge? there's quite a lot of back and forth debugging for the GitHub actions which is quite messy. Thanks!

@kccwing
Copy link
Collaborator Author

kccwing commented Aug 29, 2024

Documentation heavily referenced building this btw.

@kccwing kccwing requested a review from wong-hl August 29, 2024 08:46
.github/workflows/pypi_build.yaml Outdated Show resolved Hide resolved
.github/workflows/pypi_build.yaml Outdated Show resolved Hide resolved
.github/workflows/pypi_build.yaml Show resolved Hide resolved
.github/workflows/pypi_build.yaml Outdated Show resolved Hide resolved
.github/workflows/pypi_build.yaml Outdated Show resolved Hide resolved
.github/workflows/pypi_build_test.yaml Outdated Show resolved Hide resolved
@wong-hl
Copy link
Contributor

wong-hl commented Aug 29, 2024

I just remembered, you can test your workflows using act. I have not used it before as we had self-hosted runners, but this repo uses the GitHub's runners so it should work

@kccwing
Copy link
Collaborator Author

kccwing commented Aug 29, 2024

I just remembered, you can test your workflows using act. I have not used it before as we had self-hosted runners, but this repo uses the GitHub's runners so it should work

ohh thanks for letting me know! next time I'll do away with all these trial and error commits 😂

@kccwing
Copy link
Collaborator Author

kccwing commented Aug 29, 2024

Seems to be all working - fingers crossed this still works when we release our next version! Thanks for the review and kind suggestions @wong-hl, welcome (back) to SHARPy! 🤗

Comment on lines 30 to 31
* `pypi_build_test.yaml`: Builds the pypi wheel but does not push. Runs on changes to the `pypi*` workflows, changes to the `utils/` directory (environments) and changes to the `setup.py`. Required test for PRs to merge to `develop` and `main`.
* `pypi_build.yaml`: Builds and pushes the pypi wheel. Runs on annotated tags.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the readme?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah right, yes!! thankss

Copy link
Collaborator

@ben-l-p ben-l-p left a comment

Choose a reason for hiding this comment

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

Works for me, great to see SHARPy being made even easier to install; assuming we have no issues after merging, we can change this to be the default method of installation. Be sure to squash before merging! I will await if Hui Ling has any further feedback, then we should be ready to merge. Great contribution!

@kccwing kccwing changed the title Add hosting of built wheel at PyPI Add hosting of built wheel at PyPI - main Aug 29, 2024
@kccwing
Copy link
Collaborator Author

kccwing commented Aug 29, 2024

seems to have a slight issue with submodule folder structure - installed sharpy wants to find libxbeam.so in /xbeam/lib/, but it should have been just in /xbeam/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants