ci: tweaks to align with modern practices and jupyterhub org in general #240
Conversation
| pre-commit: | ||
| name: Run pre-commit | ||
| runs-on: ubuntu-20.04 | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| - uses: actions/setup-python@v3 | ||
| - uses: pre-commit/action@v2.0.3 | ||
|
|
There was a problem hiding this comment.
If merged, I'd enable pre-commit.ci as done across the jupyterhub org to run these tests, and more:
- create PRs to update the pre-commit-config.yaml file with modern versions of the hooks
- add commits to PRs with autoformatting changes
We have tried it out in many repos now with great success with regards to the UX for misc contributors and core developers. There is a jupyterhub team-compass issue about it for a centralized discussion: jupyterhub/team-compass#379
There was a problem hiding this comment.
To turn pre-commit.ci on for this repo, a JupyterHub github org admin should visit: https://github.com/organizations/jupyterhub/settings/installations/17782968
consideRatio
requested review from
mbmilligan and
rkdarst
and removed request for
mbmilligan
| python setup.py sdist bdist_wheel | ||
| twine upload dist/* | ||
| pip install twine | ||
| twine upload --skip-existing dist/* |
There was a problem hiding this comment.
It is my understanding that use of setup.py sdist bdist_wheel is deprecated, and we are supposed to start using a tool like build. This is what I've picked up from MinRK which seem to match what python.org documents:
| @@ -53,7 +59,7 @@ jobs: | |||
| - name: Set up Python ${{ matrix.python-version }} | |||
| uses: actions/setup-python@v3 | |||
| with: | |||
| python-version: ${{ matrix.python-version }} | |||
| python-version: "${{ matrix.python-version }}" | |||
There was a problem hiding this comment.
Let's avoid a bug ahead of time when someone has set python-version to the non-quoted string 3.10, and have it YAML parsed as 3.1, and then transformed back to a string as "3.1".
There was a problem hiding this comment.
yeah... this is one of those places where i'm very disappointed that the GHA action.yml is merely "schema-like", rather than an actual schema... though it seems like setup-python could be a bit more proactive on this.
| - name: build release | ||
| run: | | ||
| python -m build --sdist --wheel . | ||
| ls -l dist |
There was a problem hiding this comment.
might i suggest sha256sum dist/* | tee SHA256SUMS and archiving this as well?
this can provide a semi-forensic value.
| ls -l dist | |
| ls -l dist | |
| sha256sum dist/* | tee SHA256SUMS |
There was a problem hiding this comment.
Ohhh that is an interesting point. We should absolutely print this as a simple thing to do, and it will stick around for 90 days in the logs.
But how to archive it without overworking this to get it done? I wonder if you can upload this to the GitHub release as an asset or similar?
Since this is a workflow triggered by a release has been made, one could complement this with an action to upload a release artifact: https://github.com/actions/upload-release-asset#example-workflow---upload-a-release-asset
There was a problem hiding this comment.
@bollwyvl I opened jupyterhub/team-compass#502 to continue this discussion and commited your suggestion!
Co-authored-by: Nicholas Bollweg <nick.bollweg@gmail.com>
|
Being a CI only PR, and time has passed without further review, I'll go for a self-merge. Test failures were fixed in #241. |
This PR is solely about aligning the GitHub workflows with various practices across the JupyterHub org as they have evolved.
The test failures are unrelated and fixed in #241.