-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Make the release workflow more resilient #4728
Conversation
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinux
Windows
|
started a dry run: https://github.com/charliermarsh/ruff/actions/runs/5131675732 |
type: string | ||
sha: | ||
description: "Optionally, the full sha of the commit to be released" | ||
type: string |
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 don't think this is used (at least, as far as I can tell).
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.
Aren't we able to select a branch anyway in the UI?
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 can be useful in cases where you want to re-run the release workflow on a specific commit. But you're right. We could create a branch pointing to that specific commit 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.
i've changed this to be an additional check you can opt in or ignore
.github/workflows/release.yaml
Outdated
types: [ published ] | ||
inputs: | ||
tag: | ||
description: "The version to tag, without the leading 'v'. If not present, a dry run will be made" |
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.
Nit: "If omitted, will initiate a dry-run, skipping any upload steps."
push: | ||
paths: | ||
# When we change pyproject.toml, we want to ensure that the maturin builds still work | ||
- pyproject.toml |
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'd be fine to omit this and just do it manually, personally.
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 but i want tests for maturin updates :D
.github/workflows/release.yaml
Outdated
@@ -393,18 +402,29 @@ jobs: | |||
- linux-cross | |||
- musllinux | |||
- musllinux-cross | |||
if: "startsWith(github.ref, 'refs/tags/')" | |||
# If you don't set an input it's a practice run |
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.
Nit: use the "dry run" terminology here for consistency, like "If no tag was set, consider this a dry-run."
How_to_Release.md
Outdated
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.
RELEASE.md
for title consistency? Or should we just put this in CONTRIBUTING.md
, which is effectively our development guide? (It also includes benchmarking instructions, as an example.)
args: --out dist | ||
- name: "Test wheel" | ||
run: | | ||
pip install dist/${{ env.PACKAGE_NAME }}-*.whl --force-reinstall | ||
pip install --force-reinstall --find-links dist ${{ env.PACKAGE_NAME }} |
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.
What's the difference here?
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.
one works by bash globbing, the other is the pip-provided way if installing from a directory which abstract wheel naming and such away and also works if there are multiple wheels (pip selects the right one)
How_to_Release.md
Outdated
1. Attach artifacts to draft GitHub release | ||
1. Trigger downstream repositories. This can fail without causing fallout, it is possible (if | ||
inconvenient) to trigger the downstream jobs manually | ||
1. Create release notes in GitHub UI (<https://github.com/charliermarsh/ruff/releases/new>) |
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.
Does this also include promoting out of "draft"?
0aabc75
to
64a87fd
Compare
This is ready for another round of review |
RELEASING.md
Outdated
inconvenient) to trigger the downstream jobs manually | ||
1. Create release notes in GitHub UI and promote from draft to proper release(<https://github.com/charliermarsh/ruff/releases/new>) | ||
1. If needed, [update the schemastore](https://github.com/charliermarsh/ruff/blob/main/scripts/update_schemastore.py) | ||
1. If needed, update ruff-lsp and ruff-vscode |
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.
Can we put this in CONTRIBUTING.md
? There's already a small section there on releases.
Currently, it is possible to create a tag and then have the release fail, which is a problem since we can't edit the tag (#4468). This change the release process so that the tag is created inside the release workflow. This leaves as a failure mode that we have published to pypi but then creating the tag or GitHub release doesn't work, but in this case we can restart and the pypi upload is just skipped because we use the skip existing option. The release workflow is started by a workflow dispatch with the tag instead of creating the tag yourself. You can start the release workflow without a tag to do a dry run which does not publish an artifacts. You can optionally add a git sha to the workflow run and it will verify that the release runs on the mentioned commit. This also adds docs on how to release and a small style improvement for the maturin integration. Testing is hard since we can't do real releases, i've tested a minimized workflow in a separate dummy repository.
This reverts commit b4bd5a5.
This reverts commit b4bd5a5.
Summary
Currently, it is possible to create a tag and then have the release fail, which is a problem since we can't edit the tag (#4468). This change the release process so that the tag is created inside the release workflow. This leaves as a failure mode that we have published to pypi but then creating the tag or GitHub release doesn't work, but in this case we can restart and the pypi upload is just skipped because we use the skip existing option.
The release workflow is started by a workflow dispatch with the tag instead of creating the tag yourself. You can start the release workflow without a tag to do a dry run which does not publish an artifacts. You can optionally add a git sha to the workflow run and it will verify that the release runs on the mentioned commit.
This also adds docs on how to release and a small style improvement for the maturin integration.
Test Plan
Testing is hard since we can't do real releases, i've tested a minimized workflow in a separate dummy repository.