-
Notifications
You must be signed in to change notification settings - Fork 386
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
Update the CI #1204
Update the CI #1204
Conversation
Thank you for making this pull request. Did you know? You can try it on Binder: or . Also, the version of Jupytext developed in this PR can be installed with
(this requires |
71519e6
to
657d7db
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1204 +/- ##
==========================================
+ Coverage 96.74% 97.73% +0.98%
==========================================
Files 29 29
Lines 4456 4456
==========================================
+ Hits 4311 4355 +44
+ Misses 145 101 -44 ☔ View full report in Codecov by Sentry. |
abf091a
to
c6397ed
Compare
8e32403
to
54c0cfc
Compare
@LecrisUT I think I am happy with this version, are you OK if I squash and merge it? Thanks! |
Actually it will be better with a merge commit:
Let me go over the changes a bit |
- uses: actions/checkout@v4 | ||
with: | ||
ref: ${{ inputs.ref }} | ||
- uses: softprops/action-gh-release@v1 |
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.
You can add this action as well, it doesn't affect the previous workflow, it just makes sure that a release is created (as a draft) from the tag. You still need to review and push the publish release button, unless you drop the draft: true
and edit afterwards.
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 will keep that in mind, but for now I'd rather cut a release. I am not familiar with the inputs
not with the buttons in GA yet, I will need to look into that later on.
uses: ./.github/workflows/step_build.yml | ||
with: | ||
upload: true | ||
ref: ${{ inputs.ref }} |
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 one is the same workflow as you have currently, it's just separated into steps to minimize maintenance and to scope the permissions a bit
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.
Yes - I think I have the same in publish.yml
now - at least I have tried to port your changes on release.yml
there.
@@ -85,6 +87,7 @@ jobs: | |||
python -m jupyterlab.browser_check | |||
|
|||
- name: Test with pytest | |||
continue-on-error: ${{ matrix.experimental }} |
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.
Actually for this one I put the continue-on-error
only if it's on push
so that:
- when people look at the main branch they see green ticks
- when on PR we see what experimental jobs are failing and can navigate more easily
- use workflow badge to show overall pass/fail
- the pass job shows you when non-experimental jobs fail. Use that as a required job to keep trackof PR status instead
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.
That's a good suggestion, I will have to follow-up on that one
- python-version: "3.x" | ||
markdown-it-py-version: "~=2.0" | ||
markdown-it-py: "~=2.0" |
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.
Should also add ==1.0
here to track the old dependency
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.
Actually I tried ~=1.0
at https://github.com/mwouts/jupytext/actions/runs/7512100735, but the installation failed. I don't have time to do further attempts so I will go without that.
The test with --pre dependencies is experimental
2859d94
to
b1c379a
Compare
This PR follows-up on #1190