add reusable release workflow which parse pr labels #1226
Conversation
gionn
requested review from
a team,
alxgomz,
Copilot,
ffjdm,
pmacius and
siddavamshi4
There was a problem hiding this comment.
Pull request overview
This PR introduces a major refactoring of the release workflow, transitioning from a version.txt-based versioning system to a PR label-based approach. The new system automatically determines version bumps (release/major, release/minor, or release/patch) from PR labels, calculates the new semantic version, executes an optional release script, and creates GitHub releases.
- Removes
version.txtfile and associated manual version management - Adds new reusable workflow
reusable-release.ymlfor automated version bumping and release creation - Refactors
release.ymlto use the new reusable workflow - Updates
release.shto accept both current and next version as parameters - Updates documentation and coding guidelines to reflect the label-based release process
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
version.txt |
Removed - versioning now handled via PR labels and git tags |
docs/README.md |
Added documentation for new reusable-release workflow and updated release process instructions |
.github/workflows/reusable-release.yml |
New reusable workflow that parses PR labels, calculates semantic versions, and creates releases |
.github/workflows/release.yml |
Refactored to use the new reusable workflow with label-based or manual version selection |
.github/scripts/release.sh |
Updated to accept both new and current version parameters for reference updates |
.github/copilot-instructions.md |
Updated guidelines to reflect label-based release process instead of version.txt |
Comments suppressed due to low confidence (1)
.github/scripts/release.sh:25
- Potential issue with
grep -Rlwhen no files match the pattern. If there are no files containing the patternAlfresco/alfresco-build-tools.*@$CURRENT_VERSION,grep -Rlwill return a non-zero exit code, causing the script to fail due toset -e. Consider adding a fallback or usinggrep -Rl ... || trueto handle cases where no files match (e.g., first release or all references already updated).
| release: | ||
| name: Release | ||
| if: github.event.pull_request.merged == true || github.event_name == 'workflow_dispatch' | ||
| uses: Alfresco/alfresco-build-tools/.github/workflows/[email protected] |
There was a problem hiding this comment.
The documentation example shows usage with version tag @v10.3.0, but according to the custom guideline, when referencing internal actions in workflow YAML files, you should use the latest released tag. Since this PR is proposing a major version bump (indicated by the checkbox in the PR description), the current latest would be v10.4.0, not v10.3.0.
However, per the guidelines, you should not update this reference to the next release version in the PR itself, as the tag will only exist after the release. This should use the current latest released tag, which appears to be v10.4.0 based on the version.txt content in the main branch.
|
|
||
| Bump version defined in [version.txt](/version.txt) during a PR, release | ||
| workflow is triggered automatically on PR close. | ||
| Add a label to the PR among `major`, `minor`, or `patch` to trigger a release |
There was a problem hiding this comment.
Inconsistent label format in documentation. The reusable workflow looks for labels with the release/ prefix (release/major, release/minor, release/patch), but the documentation here states "Add a label to the PR among major, minor, or patch" without the prefix. This should be updated to match the actual implementation: "Add a label to the PR among release/major, release/minor, or release/patch".
| Add a label to the PR among `major`, `minor`, or `patch` to trigger a release | |
| Add a label to the PR among `release/major`, `release/minor`, or `release/patch` to trigger a release |
| commit_user_name: ${{ inputs.commit_username }} | ||
| commit_user_email: ${{ inputs.commit_email }} |
There was a problem hiding this comment.
The commit username and email inputs are optional but are used unconditionally in the git-auto-commit-action. If these inputs are not provided, the action will receive empty values which may cause issues with git commits. Consider either making these inputs required, providing default values, or conditionally using the action only when both username and email are provided.
| - name: Run optional release script | ||
| if: steps.set-release-level.outputs.release-level != '' && inputs.release_script != '' | ||
| run: | | ||
| ${{ inputs.release_script }} "${{ steps.bump-semver.outputs.new-version }}" "${{ steps.bump-semver.outputs.current-version }}" |
There was a problem hiding this comment.
Two issues with the release script execution:
-
Security: The release script path is passed directly to the shell without validation. Consider adding validation to ensure the script path starts with a safe prefix (e.g.,
./.github/scripts/) to prevent execution of arbitrary scripts. -
Execution permissions: The script is executed directly without ensuring it has execute permissions. Consider using
bash ${{ inputs.release_script }} ...instead of direct execution to avoid permission issues and improve portability.
| ${{ inputs.release_script }} "${{ steps.bump-semver.outputs.new-version }}" "${{ steps.bump-semver.outputs.current-version }}" | |
| # Validate that the release script path starts with ./.github/scripts/ | |
| if [[ "${{ inputs.release_script }}" != ./.github/scripts/* ]]; then | |
| echo "::error::release_script must start with ./.github/scripts/" | |
| exit 1 | |
| fi | |
| # Run the script using bash for portability and to avoid permission issues | |
| bash "${{ inputs.release_script }}" "${{ steps.bump-semver.outputs.new-version }}" "${{ steps.bump-semver.outputs.current-version }}" |
| if [[ "$LATEST_TAG" =~ ^v?([0-9]+)\.([0-9]+)\.([0-9]+)(-.+)?$ ]]; then | ||
| MAJOR="${BASH_REMATCH[1]}" | ||
| MINOR="${BASH_REMATCH[2]}" | ||
| PATCH="${BASH_REMATCH[3]}" | ||
| else | ||
| MAJOR=0 | ||
| MINOR=0 | ||
| PATCH=0 | ||
| fi |
There was a problem hiding this comment.
[nitpick] The regex pattern allows for optional pre-release suffixes with (-.+)? but this suffix is discarded when parsing the version components. If the current version has a pre-release suffix (e.g., v1.2.3-beta), the bumping logic will treat it as v1.2.3 and bump from there. This may be intentional, but it should be documented or handled explicitly to avoid confusion. Consider whether pre-release versions should trigger a warning or special handling.
| secrets: | ||
| BOT_GITHUB_TOKEN: ${{ secrets.BOT_GITHUB_TOKEN }} | ||
| ``` | ||
|
|
There was a problem hiding this comment.
The documentation example is missing the release_script input parameter that is used in the actual implementation (.github/workflows/release.yml line 27). While this parameter is optional, the example should either include it to show users how to use it, or add a note explaining that an optional release_script parameter is available for custom release logic.
| **Note**: The reusable release workflow also supports an optional `release_script` input parameter for custom release logic. You can provide a shell script path to override the default release steps. See the workflow documentation for details. |
Checklist
Description
This pull request refactors the release workflow to use a new reusable workflow, making the release process more modular, configurable, and easier to maintain. The versioning is now handled via PR labels.