-
-
Notifications
You must be signed in to change notification settings - Fork 3
ci(release): Switch from action-prepare-release to Craft #527
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
base: main
Are you sure you want to change the base?
Conversation
This PR migrates from the deprecated action-prepare-release to the new Craft GitHub Actions (reusable workflow or composite action). Changes: - Migrate .github/workflows/release.yml to Craft reusable workflow - Update .craft.yml with versioning.policy: calver
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Build / dependencies / internal 🔧
Other
🤖 This preview updates automatically when you update the PR. |
.github/workflows/release.yml
Outdated
| version: ${{ inputs.version }} | ||
| force: ${{ inputs.force }} |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| pull_request: | ||
| types: | ||
| - opened | ||
| - synchronize | ||
| - reopened | ||
| - edited | ||
| - labeled |
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.
Why? This is going to be noise most of the time. Taskbroker follows calver.
The previous migration incorrectly removed the GitHub App token authentication step. This commit restores it by switching to the composite action pattern which preserves the auth flow.
.github/workflows/release.yml
Outdated
| version: ${{ inputs.version }} | ||
| force: ${{ inputs.force }} |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
.github/workflows/release.yml
Outdated
| version: ${{ inputs.version }} | ||
| force: ${{ inputs.force }} |
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.
Bug: The scheduled release workflow may fail because it passes an empty string to the version input of the craft action, which might not be handled correctly.
Severity: HIGH
Suggested Fix
To ensure the scheduled job runs correctly, provide a default value for the version input. Setting default: "auto" for the version input would make the behavior explicit for scheduled runs and align with the input's description.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/release.yml#L37-L38
Potential issue: The `release.yml` workflow is triggered by both manual
`workflow_dispatch` and an automated `schedule`. When the workflow runs on its schedule,
the `inputs` context is not populated. As a result, the `version` and `force` parameters
passed to the `getsentry/craft` action will be empty strings. It is unknown how the
`craft` action handles an empty string for the `version` input. If it does not default
to an automatic versioning scheme (like calver) when given an empty string, the
scheduled release process will fail.
Did we get this right? 👍 / 👎 to inform future reviews.
.github/workflows/release.yml
Outdated
| version: ${{ inputs.version }} | ||
| force: ${{ inputs.force }} |
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.
Bug: The workflow runs on a schedule which does not provide inputs. Using ${{ inputs.version }} and ${{ inputs.force }} will result in undefined values during scheduled runs.
Severity: HIGH
Suggested Fix
Provide default values for the inputs to ensure they are defined during scheduled runs. For example, use ${{ inputs.version || 'auto' }}. This explicitly handles the case where the input is not provided, aligning with the likely intended behavior of auto-versioning for scheduled releases.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/release.yml#L37-L38
Potential issue: The `release.yml` workflow is triggered by both `workflow_dispatch` and
`schedule`. The `workflow_dispatch` event provides `inputs.version` and `inputs.force`,
but the `schedule` event does not. When the workflow runs on its schedule, the values
passed to the `craft` action for `version` and `force` will be empty or undefined. While
the `craft` action might be configured to handle this by auto-generating a version, this
behavior is unconfirmed. If the action does not gracefully handle empty inputs, the
scheduled release process could fail.
Did we get this right? 👍 / 👎 to inform future reviews.
Summary
This PR migrates from the deprecated
action-prepare-releaseto the new Craft GitHub Actions.Changes
.github/workflows/release.ymlto Craft reusable workflow.craft.ymlwithversioning.policy: calverDocumentation
See https://getsentry.github.io/craft/github-actions/ for more information.