Conversation
Zach-Attach
left a comment
Zach-Attach
left a comment
There was a problem hiding this comment.
I really like this. Should be a huge time saver, but I see a few issues that need to be addressed.
| python -m pip install setuptools==65.5.0 pip==21 | ||
| - name: Install pypa/build | ||
| run: | | ||
| python3 -m pip install build --user |
There was a problem hiding this comment.
Why does this change from python to python3? Seems inconsistent
There was a problem hiding this comment.
I agree, let's make it python3
| publish-to-pypi: | ||
| name: >- | ||
| Publish Python 🐍 distribution 📦 to PyPI | ||
| if: startsWith(github.ref, 'refs/tags/') # only publish to PyPI on tag pushes |
There was a problem hiding this comment.
So this is triggered by pushing a new tag? Does this mean we still need to push a new tag?
Should this not be triggered by a push to main instead? And then we can grab the version from src/nett/_version.py.
This is how I would imagine it:
- Check if this is a PR to main AND that the version found in src/nett/_version.py is not an existing release (denoted as v{version num}): If both are true, run the build to confirm it is buildable.
- If this is being pushed to main (from merging the PR): Start the publication process, resulting in it being tagged and published to pypi.
The particular if statement solution for seeing if it is being pushed to main or is just a PR for main can be found in the docs workflow, see https://github.com/buildingamind/NewbornEmbodiedTuringTest/blob/bf29953ab909ae216eb67a06fb9dbda281b9def1/.github/workflows/docs.yml#L3C1-L11C12
There was a problem hiding this comment.
To be honest I don't have an opinion, I'm fine if we do this either way, but if what you have is already how docs work, it would make all the sense to have consistency between the two workflows.
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.10.12" | ||
| - name: Install specific setuptools and pip |
There was a problem hiding this comment.
Minor suggestion (feel free to ignore): Caching would greatly help with build times here. See: https://github.com/buildingamind/NewbornEmbodiedTuringTest/blob/bf29953ab909ae216eb67a06fb9dbda281b9def1/.github/workflows/docs.yml#L27C6-L42C47
There was a problem hiding this comment.
For bonus points, building this once and then using it for both docs.yml and publish-to-test.yml would be nice and efficient.
| runs-on: ubuntu-latest | ||
| environment: | ||
| name: pypi | ||
| url: https://pypi.org/p/nett-benchmarkse |
There was a problem hiding this comment.
Typo:
| url: https://pypi.org/p/nett-benchmarkse | |
| url: https://pypi.org/p/nett-benchmarks |
There was a problem hiding this comment.
Good catch, thanks
|
Just looking at this. Do you think there are too many checks? Also, these names are kinda confusing lol |
|
I've made some changes, and I'm testing out the workflow file with testpypi, I'll let keep this updated |
Yeah I think so too, but they are doing useful stuff so I'll let it be for now. I want to have a version of this working and then we can move things around |
2 participants
This PR address #125 , and adds a GitHub Workflow for automatically pushing a tagged release to PyPI.