RDKEMW-10094: Update component-release.yml #167
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds PR description validation to the component release workflow, ensuring that a version bump type is specified before a PR can be merged. The validation job runs on PR events (opened, edited, ready_for_review) and checks that the PR description contains a "version: major|minor|patch" field.
Key Changes
- Added three new PR event types (opened, edited, ready_for_review) to trigger the workflow
- Introduced a
validate-versionjob that enforces version field presence in PR descriptions - Validation runs early in the PR lifecycle to provide immediate feedback
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| jobs: | ||
| validate-version: | ||
| if: github.event.action == 'opened' || github.event.action == 'edited' || github.event.action == 'ready_for_review' |
There was a problem hiding this comment.
The condition check should use github.event.pull_request.merged != true as well to avoid running validation on closed/merged PRs. When a PR is closed (merged or not), the 'closed' event is triggered but this condition doesn't explicitly exclude it. Consider using if: github.event.pull_request.merged != true or ensure the job doesn't run for closed events.
| if: github.event.action == 'opened' || github.event.action == 'edited' || github.event.action == 'ready_for_review' | |
| if: (github.event.action == 'opened' || github.event.action == 'edited' || github.event.action == 'ready_for_review') && github.event.pull_request.merged != true |
| - name: Validate PR description for version field | ||
| run: | | ||
| PR_DESC="${{ github.event.pull_request.body }}" | ||
| if ! echo "$PR_DESC" | grep -qiE 'version[[:space:]]*:[[:space:]]*(major|minor|patch)'; then |
There was a problem hiding this comment.
The regex pattern used for validation differs from the one used in the release job (line 77). The validation pattern uses [[:space:]]* while the release job uses \s*. Consider using the same pattern in both places for consistency: 'version\s*:\s*(major|minor|patch)'.
| if ! echo "$PR_DESC" | grep -qiE 'version[[:space:]]*:[[:space:]]*(major|minor|patch)'; then | |
| if ! echo "$PR_DESC" | grep -qiE 'version\s*:\s*(major|minor|patch)'; then |
| - name: Validate PR description for version field | ||
| run: | | ||
| PR_DESC="${{ github.event.pull_request.body }}" | ||
| if ! echo "$PR_DESC" | grep -qiE 'version[[:space:]]*:[[:space:]]*(major|minor|patch)'; then |
There was a problem hiding this comment.
The PR description is not properly escaped when used in the shell command. If the PR description contains special characters or newlines, it could cause unexpected behavior or command injection. Consider using a heredoc or passing the value through an environment variable instead: echo \"$PR_DESC\" | grep -qiE ...
| if ! echo "$PR_DESC" | grep -qiE 'version[[:space:]]*:[[:space:]]*(major|minor|patch)'; then | |
| if ! grep -qiE 'version[[:space:]]*:[[:space:]]*(major|minor|patch)' <<EOF | |
| $PR_DESC | |
| EOF | |
| then |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for change: Enforce the version field in PR description
Test Procedure: Refer ticket
Risks: Low
Version: Patch
Signed-off-by: Soundaryaa Sitaram[email protected]