-
Notifications
You must be signed in to change notification settings - Fork 388
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
ci: Minor reorganizations #1190
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
635c3b6
Remove concurrency
LecrisUT 1ed9ab2
Update action versions
LecrisUT 5e0bc6f
Reorganize test dependencies
LecrisUT 773b977
Harden the workflows
LecrisUT 1f1d580
Update python-version defaults
LecrisUT c3bba69
Remove explicit `markdown-it-py`
LecrisUT afd6891
Simplify release CI
LecrisUT 6db16ba
Improve CI display
LecrisUT 332fc1a
Add pass action
LecrisUT 57176f7
Simplify step_build
LecrisUT 5145497
Remove the sdist.exclude
LecrisUT bea7f2c
Remove jupyterlab upper-bound
LecrisUT 4927d5f
Add missing `.git_archival.txt`
LecrisUT a2f8106
Configure `fail-fast` option
LecrisUT e5dcfa6
Bump minimum `markdown-it-py`
LecrisUT b43b625
fix `markdown-it-py-version` CI
LecrisUT dc3b9fd
Package name convention
LecrisUT 0ecd51f
Configure release CD
LecrisUT 269d225
Run pytest in parallel
LecrisUT 113d2c2
Reconfigure codecov
LecrisUT 1870d19
Conditionally upload code coverage
LecrisUT File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
node: $Format:%H$ | ||
node-date: $Format:%cI$ | ||
describe-name: $Format:%(describe:tags=true,match=*[0-9]*)$ | ||
ref-names: $Format:%D$ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
World\ population.ipynb linguist-documentation | ||
.git_archival.txt export-subst |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,6 @@ | ||
codecov: | ||
notify: | ||
after_n_builds: 16 | ||
|
||
comment: | ||
after_n_builds: 16 | ||
wait_for_ci: true | ||
|
||
coverage: | ||
status: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
name: Release | ||
on: | ||
push: | ||
tags: | ||
- "v[0-9]+.[0-9]+.[0-9]+" | ||
- "v[0-9]+.[0-9]+.[0-9]+rc[0-9]+" | ||
- "v[0-9]+.[0-9]+.[0-9]+dev[0-9]+" | ||
workflow_dispatch: | ||
inputs: | ||
skip-tests: | ||
type: boolean | ||
description: Skip | ||
default: false | ||
ref: | ||
type: string | ||
description: Tag to release | ||
required: true | ||
|
||
permissions: | ||
contents: read | ||
|
||
jobs: | ||
pre-commit: | ||
uses: ./.github/workflows/step_pre-commit.yml | ||
if: "! inputs.skip-tests" | ||
|
||
test-pip: | ||
needs: [ pre-commit ] | ||
uses: ./.github/workflows/step_tests-pip.yml | ||
with: | ||
coverage: false | ||
if: "! inputs.skip-tests" | ||
|
||
test-conda: | ||
needs: [ test-pip ] | ||
uses: ./.github/workflows/step_tests-conda.yml | ||
with: | ||
coverage: false | ||
if: "! inputs.skip-tests" | ||
|
||
test-ui: | ||
needs: [ test-pip ] | ||
uses: ./.github/workflows/step_tests-ui.yml | ||
if: "! inputs.skip-tests" | ||
|
||
test-status: | ||
needs: [ pre-commit, test-pip, test-conda, test-ui ] | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: re-actors/alls-green@release/v1 | ||
with: | ||
allowed-skips: pre-commit, test-pip, test-conda, test-ui | ||
jobs: ${{ toJSON(needs) }} | ||
if: always() | ||
|
||
build: | ||
needs: [ test-status ] | ||
uses: ./.github/workflows/step_build.yml | ||
with: | ||
upload: true | ||
ref: ${{ inputs.ref }} | ||
|
||
publish: | ||
needs: [ build ] | ||
runs-on: ubuntu-latest | ||
|
||
environment: | ||
name: pypi | ||
url: https://pypi.org/p/jupytext | ||
|
||
permissions: | ||
contents: read | ||
id-token: write | ||
|
||
steps: | ||
- uses: actions/download-artifact@v3 | ||
with: | ||
name: dist | ||
path: dist | ||
- name: Publish | ||
uses: pypa/gh-action-pypi-publish@release/v1 | ||
|
||
release: | ||
needs: [ publish ] | ||
name: Create release | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v4 | ||
with: | ||
ref: ${{ inputs.ref }} | ||
- uses: softprops/action-gh-release@v1 | ||
with: | ||
name: Jupytext ${{ inputs.ref || github.ref_name }} | ||
draft: true | ||
prerelease: ${{ contains(inputs.ref || github.ref, 'rc') }} | ||
generate_release_notes: true | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 changes: 11 additions & 12 deletions
23
...b/workflows/step_test_unit_functional.yml → .github/workflows/step_coverage.yml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Instead of reinventing the wheel, I would suggest to look into jupyter-releaser. It will work out-of-the-box nicely to publish to both PyPI and NPM.
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.
Sure, open to suggestions on this one, can you propose the change so I can look at what it does and check if it's equivalent? I'm curious if it works with
Trusted publishing
and how that works when being used with both PyPI and NPMThere 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.
It does work with
Trusted publishing
which is very neat. For NPM we still need to use a token as a secret to publish.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 need to read it carefully, because the workflow seems incompatible:
https://github.com/jupyter-server/jupyter_releaser/blob/43d510770f1fe039e133890936416956b927e250/jupyter_releaser/actions/populate_release.py#L42
(This workflow runs on tag release, while the
jupyter_releasser
creates the new tag) 🤔. Indeed there are good points like the bumpimg ofpackage.json
, but then how do you specify the tag?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.
This is a manual workflow. So, the maintainer will kickstart a release process by prepping a release -> https://jupyter-releaser.readthedocs.io/en/latest/get_started/making_release_from_repo.html#prep-release
And this will produce the artifacts and updated changelog. Once the maintainer is happy with everything, they can go ahead with release -> https://jupyter-releaser.readthedocs.io/en/latest/get_started/making_release_from_repo.html#publish-release
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.
Ok, I think we have two possible workflows here:
pre-release
to bump to the next version. I think we can mix individual steps from the releaser as long as we order them appropriatelyIt is a bit weird because of the mixing of dynamic-version via git on the python sideand the static version of npm. Normally I prefer to bump versions after a release in order to clearly communicate the difference in versions (e.g. main vs tag release)
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.
Well, my original suggestion was to replace the current
release.yml
file with the ones from jupyter-releaser instead of mixing them with current one. But I see few potential blockers adoptingjupyter-releaser
:jupyter-releaser
expectspackage.json
to be in root of the repo for it to build and release NPM packages.We do not use dynamic version (if you mean like
versioneer
creating a new version string for each commit). What hatch will do is bump NPM version inpackage.json
and then make sure that Python package will use that same version aspackage.json
. Currently, versions of NPM package and Python package of Jupytext are diverged and hence, it does not work out-of-the-box.Your suggestion of creating our own workflows based on
jupyter-releaser
might work. Something like splitting like these workflows for Python and NPM packages separately. But that increases the maintainence burden. As @mwouts is the one who is making releases, it depends on how much complexity he is willing to handle.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.
Slight nuance because you still need a
release.yml
with anon:workflow_dispatch
. But yeah, what you suggest is about the second approach in my last comment. We just need to have a light wrapper to forward the inputs.Ah you're right
src/jupytext/version.py
is a static file. I confused it withtool.hatch.build.hooks.vcs.version-file
. Would the version bumping work on the python side?Good point. In either case it needs to re-converge the versioning number, unless there are more complicated tag forms.
Indeed, looking back there are quite a few changes necessary. And in either case we need to test these workflows because there seems to be quite a few things to look out for. @mahendrapaipuri can the
jupyter-releaser
be tested locally on a fork, like uploading to GitHub package repo. And indeed the main question is @mwouts, which one do you prefer: tag push, calling workflow, or manual commits?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.
Ah, I thought we are already using
hatch
to do version bumping. But currenctly version string is set manually I assume. Yes, we can do the bumping usinghatch
Yes, there is a check-release workflow that dry runs all the release steps on local PyPI and mock GitHub instance in CI. So, if and when we decide to use
jupyter-releaser
tools in release workflows, we can adopt a similar solution to test it locally on mock instances.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.
Thanks @LecrisUT and @mahendrapaipuri for this. I am afraid this is too unfamiliar to me. I am happy with the current process for releasing, i.e. editing the
CHANGELOG.md
and version files manually, and then adding a tag in the GitHub interface, so if you don't mind I might left this change for later and revert to the previous state ofpublish.yml
for now.