-
Notifications
You must be signed in to change notification settings - Fork 194
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(workflows): add preview support in main release #3007
Conversation
f674b85
to
af6ce49
Compare
WalkthroughOhayo sensei! This PR removes the GitHub Actions workflow file Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
5-9
: Ohayo sensei! Consider changing the default value for preview.Setting
preview
totrue
by default means every manual workflow run will create a preview release unless explicitly set to false. This might lead to accidental preview releases. Consider changing the default tofalse
to ensure preview releases are created only when explicitly requested.preview: description: 'Create a preview release' type: boolean - default: true + default: false
213-213
: Improve Docker image tagging strategy.For preview releases, we should avoid updating the
latest
tag to prevent confusion.- tags: ghcr.io/${{ github.repository }}:latest,ghcr.io/${{ github.repository }}:${{ needs.prepare.outputs.tag_name }} + tags: ${{ !startsWith(needs.prepare.outputs.tag_name, 'preview') && format('ghcr.io/{0}:latest,ghcr.io/{0}:{1}', github.repository, needs.prepare.outputs.tag_name) || format('ghcr.io/{0}:{1}', github.repository, needs.prepare.outputs.tag_name) }}
🛑 Comments failed to post (2)
.github/workflows/release.yml (2)
170-170:
⚠️ Potential issueFix YAML syntax error in release condition.
The condition syntax is incorrect. In YAML, expressions should be properly quoted.
- if: !startsWith(needs.prepare.outputs.tag_name, 'preview') # only trigger a release for non-preview tags + if: "!startsWith(needs.prepare.outputs.tag_name, 'preview')" # only trigger a release for non-preview tags📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if: "!startsWith(needs.prepare.outputs.tag_name, 'preview')" # only trigger a release for non-preview tags
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 170-170: syntax error: expected , but found ''
(syntax)
31-36: 🛠️ Refactor suggestion
Improve version determination logic.
- The condition
${{ github.event.inputs.preview }}
will be undefined for PR triggers, which could lead to unexpected behavior.- The version prefix 'v' is used inconsistently across the workflow.
- if ${{ github.event.inputs.preview }}; then + if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ github.event.inputs.preview }}" == "true" ]]; then echo "tag_name=preview--$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT else cargo install cargo-get - echo "tag_name=v$(cargo get workspace.package.version)" >> $GITHUB_OUTPUT + echo "tag_name=$(cargo get workspace.package.version)" >> $GITHUB_OUTPUT fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ github.event.inputs.preview }}" == "true" ]]; then echo "tag_name=preview--$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT else cargo install cargo-get echo "tag_name=$(cargo get workspace.package.version)" >> $GITHUB_OUTPUT fi
af6ce49
to
5553f52
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3007 +/- ##
==========================================
- Coverage 56.62% 56.61% -0.02%
==========================================
Files 431 431
Lines 57637 57637
==========================================
- Hits 32637 32631 -6
- Misses 25000 25006 +6 ☔ View full report in Codecov by Sentry. |
5553f52
to
8de44d9
Compare
…ease Removed the `preview.yml` workflow as its functionality is now integrated into the `release.yml` workflow. Added support for creating preview releases via an input flag and adjusted related steps for tag naming and conditions in the release process.
8de44d9
to
0459f89
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
31-36
: Consider enhancing the preview tag format for better traceability.While the current format works, we could make it more informative by including additional context.
Consider this enhanced format:
- echo "tag_name=preview--$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT + echo "tag_name=preview-$(date +%Y%m%d)-$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUTThis change would:
- Add a date stamp for chronological tracking
- Make it easier to identify when the preview was created
213-213
: Consider using a different latest tag for preview builds.To avoid confusion, it would be better to use a different latest tag for preview builds.
Consider this modification:
- tags: ghcr.io/${{ github.repository }}:latest,ghcr.io/${{ github.repository }}:${{ needs.prepare.outputs.tag_name }} + tags: ${{ github.event.inputs.preview == 'true' && format('ghcr.io/{0}:preview-latest,ghcr.io/{0}:{1}', github.repository, needs.prepare.outputs.tag_name) || format('ghcr.io/{0}:latest,ghcr.io/{0}:{1}', github.repository, needs.prepare.outputs.tag_name) }}This ensures that:
- Preview builds use
preview-latest
tag- Regular builds continue to use
latest
tag
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/preview.yml
(0 hunks).github/workflows/release.yml
(8 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/preview.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
172-172: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
.github/workflows/release.yml (2)
5-9
: Ohayo sensei! The preview input parameter is well-configured.The boolean input parameter with a clear description and sensible default value aligns well with the PR's objective of adding preview support.
170-170
: The release condition correctly handles preview builds.The condition ensures that GitHub releases are only created for non-preview builds, which is the expected behavior.
@@ -158,10 +167,10 @@ jobs: | |||
env: | |||
GITHUB_USER: ${{ github.repository_owner }} | |||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | |||
|
|||
if: ${{ !startsWith(needs.prepare.outputs.tag_name, 'preview') }} | |||
steps: | |||
- uses: actions/checkout@v2 |
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.
🛠️ Refactor suggestion
Update the checkout action version.
The checkout action version is outdated and should be updated to match other actions.
Apply this change:
- - uses: actions/checkout@v2
+ - uses: actions/checkout@v4
This update:
- Aligns with other action versions in the workflow
- Addresses the actionlint warning
- Ensures compatibility with current GitHub Actions runners
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- uses: actions/checkout@v2 | |
- uses: actions/checkout@v4 |
🧰 Tools
🪛 actionlint (1.7.4)
172-172: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
31-36
: Add error handling for version determination.While the logic is correct, consider adding error handling:
- Validate cargo-get installation success
- Ensure workspace version follows semver format
if ${{ github.event.inputs.preview }}; then echo "tag_name=preview--$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT else - cargo install cargo-get - echo "tag_name=v$(cargo get workspace.package.version)" >> $GITHUB_OUTPUT + if ! cargo install cargo-get; then + echo "::error::Failed to install cargo-get" + exit 1 + fi + VERSION=$(cargo get workspace.package.version) + if ! echo "$VERSION" | grep -E '^[0-9]+\.[0-9]+\.[0-9]+'; then + echo "::error::Invalid version format: $VERSION" + exit 1 + fi + echo "tag_name=v$VERSION" >> $GITHUB_OUTPUT fi
191-211
: Update Docker-related actions to latest versions.Several Docker-related actions are using older versions.
Apply these updates:
- uses: actions/checkout@v2 + uses: actions/checkout@v4 - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v3 - uses: docker/login-action@v1 + uses: docker/login-action@v3 - uses: docker/build-push-action@v3 + uses: docker/build-push-action@v5🧰 Tools
🪛 actionlint (1.7.4)
191-191: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
196-196: property "job" is not defined in object type {}
(expression)
200-200: the runner of "docker/setup-buildx-action@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
203-203: the runner of "docker/login-action@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
210-210: the runner of "docker/build-push-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(7 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
172-172: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
175-175: property "job" is not defined in object type {}
(expression)
196-196: property "job" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: x86_64-pc-windows-msvc (windows-latest)
- GitHub Check: aarch64-apple-darwin (macos-latest)
- GitHub Check: x86_64-apple-darwin (macos-latest-xlarge)
- GitHub Check: aarch64-unknown-linux-gnu (ubuntu-latest-8-cores-arm64)
- GitHub Check: x86_64-unknown-linux-gnu (ubuntu-latest-8-cores)
- GitHub Check: ensure-wasm
- GitHub Check: docs
- GitHub Check: clippy
🔇 Additional comments (2)
.github/workflows/release.yml (2)
5-9
: Ohayo sensei! Nice input parameter definition!The preview input parameter is well-structured with clear description, appropriate type, and sensible default value.
172-172
: Update the checkout action version.The checkout action version is outdated and should be updated to match other actions.
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4🧰 Tools
🪛 actionlint (1.7.4)
172-172: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
with: | ||
name: binaries | ||
name: binaries-${{ matrix.job.target }} |
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.
Fix matrix references in artifact steps.
The matrix.job references are undefined as these steps are not within a matrix strategy context.
These should reference specific artifact names or use a different naming strategy. Consider:
- Using static names
- Using environment variables
- Using job outputs
Example fix:
-name: binaries-${{ matrix.job.target }}
+name: binaries-release
Also applies to: 175-175, 196-196
dd76980
to
68bca84
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
31-36
: Add error handling for version retrieval.While the logic is correct, we should handle potential failures in version retrieval.
if ${{ github.event.inputs.preview }}; then echo "tag_name=preview--$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT else cargo install cargo-get - echo "tag_name=v$(cargo get workspace.package.version)" >> $GITHUB_OUTPUT + VERSION=$(cargo get workspace.package.version) + if [ -z "$VERSION" ]; then + echo "Error: Failed to retrieve version from Cargo.toml" + exit 1 + fi + echo "tag_name=v${VERSION}" >> $GITHUB_OUTPUT fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(6 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
167-167: label "ubuntu-20.04-4-cores" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
174-174: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: clippy
- GitHub Check: ensure-wasm
- GitHub Check: docs
- GitHub Check: x86_64-pc-windows-msvc (windows-latest)
- GitHub Check: aarch64-apple-darwin (macos-latest)
- GitHub Check: x86_64-apple-darwin (macos-latest-xlarge)
- GitHub Check: aarch64-unknown-linux-gnu (ubuntu-latest-8-cores-arm64)
- GitHub Check: x86_64-unknown-linux-gnu (ubuntu-latest-8-cores)
🔇 Additional comments (4)
.github/workflows/release.yml (4)
5-9
: Ohayo sensei! LGTM on the preview input parameter!The new input parameter is well-defined with appropriate type and default value.
151-156
: Fix matrix references in artifact steps.The matrix.job references are undefined as these steps are not within a matrix strategy context.
174-174
: Update the checkout action version.🧰 Tools
🪛 actionlint (1.7.4)
174-174: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
216-216
: Ohayo! LGTM on the Docker image tagging!The tag references correctly use the new tag_name output, supporting both preview and regular releases.
Updated artifact and binary names in the release workflow to include matrix job targets for improved identification and clarity in multi-target builds.
68bca84
to
e15a2f8
Compare
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/release.yml (1)
165-165
: 🛠️ Refactor suggestionUpdate runner label and verify the preview condition.
The preview condition looks good, but the runner label needs updating.
Apply this diff:
-runs-on: ubuntu-20.04-4-cores +runs-on: ubuntu-latest-4-coresAlso applies to: 170-170
🧰 Tools
🪛 actionlint (1.7.4)
165-165: label "ubuntu-20.04-4-cores" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-22.04", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "macos-12-xl", "macos-12-xlarge", "macos-12-large", "macos-12", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
♻️ Duplicate comments (2)
.github/workflows/release.yml (2)
151-161
:⚠️ Potential issueFix matrix references in artifact names.
While the version updates to v4 are good, the matrix references need fixing.
Apply these diffs:
-name: binaries-${{ matrix.job.target }} +name: binaries-${{ matrix.job.platform }}-${{ matrix.job.arch }}-name: artifacts-${{ matrix.job.target }} +name: artifacts-${{ matrix.job.platform }}-${{ matrix.job.arch }}-pattern: artifacts-* +pattern: artifacts-*-*Also applies to: 173-176, 194-199
191-191
:⚠️ Potential issueUpdate checkout action version.
The Docker tag changes look good, but the checkout action needs updating.
Apply this diff:
-uses: actions/checkout@v2 +uses: actions/checkout@v4Also applies to: 214-214
🧰 Tools
🪛 actionlint (1.7.4)
191-191: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
31-36
: Enhance tag name logic for better clarity and reliability.Consider these improvements:
- Make preview tags more descriptive by including PR number or branch name.
- Add error handling for cargo-get installation.
Apply this diff:
if ${{ github.event.inputs.preview }}; then - echo "tag_name=preview--$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT + echo "tag_name=preview-${{ github.event.pull_request.number || 'manual' }}-$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT else - cargo install cargo-get + cargo install cargo-get || { echo "Failed to install cargo-get"; exit 1; } echo "tag_name=v$(cargo get workspace.package.version)" >> $GITHUB_OUTPUT fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(7 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/release.yml
172-172: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (2)
.github/workflows/release.yml (2)
5-9
: Ohayo sensei! The preview input parameter looks good!The input parameter is well-defined with a clear description and appropriate type.
121-121
: Ohayo! The version name change looks good!The VERSION_NAME now correctly uses the tag_name output, maintaining consistency with the new tag naming strategy.
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.
Not sure how to test if not on main, thanks @steebchen for the modifications here.
This enables us to use cross-platform images in preview builds.
Summary by CodeRabbit