RDKEMW-22790: Commit message validator change - #576
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the GitHub Actions PR metadata validator to align PR titles/descriptions with the “Commit Message Format For RDKE” requirements, so merge metadata is consistently structured across the repo.
Changes:
- Replaces the prior “Summary/Type/Test Plan/Jira” body checks with a new commit-message-format-based template.
- Adds PR title validation for
<JIRA-1234>: <summary>and a consolidated PR body validation regex.
Suppressed comments (1)
.github/workflows/commit_message_format.yml:75
- The PR body regex uses constructs that are not compatible with JavaScript RegExp (used by actions-ecosystem/action-regex-match): the inline modifier
(?s)is invalid in JS, and\Ris not a recognized escape (it will be treated as a literal 'R'), so this check will likely never match (or error). Also, the current regex makesTest Procedure:optional even though the workflow message states it is required.
# Check PR body
- name: Match PR Body
uses: actions-ecosystem/action-regex-match@v2
id: pr-body-match
if: failure() || success()
with:
text: ${{ github.event.pull_request.body }}
regex: (?s)^([A-Z][A-Z0-9]+-\d+): (.{1,64})\R\RReason for change:.*?(?:\RTest Procedure:.*)?$
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Check PR title | ||
| - name: Match PR title | ||
| uses: actions-ecosystem/action-regex-match@v2 | ||
| id: summary-match | ||
| id: pr-title-match | ||
| if: failure() || success() | ||
| with: | ||
| text: ${{ github.event.pull_request.body }} | ||
| regex: 'Summary:\s*[\s\S]*?' | ||
| flags: gm | ||
|
|
||
| - name: Check 'Summary:' | ||
| if: (failure() || success()) && (steps.summary-match.outputs.match == '') | ||
| run: exit 1 | ||
|
|
||
| # Check PR description for 'Type: Feature|Fix|Cleanup' field. | ||
| - name: Match 'Type:' | ||
| uses: actions-ecosystem/action-regex-match@v2 | ||
| id: type-match | ||
| if: failure() || success() | ||
| with: | ||
| text: ${{ github.event.pull_request.body }} | ||
| regex: 'Type:\s*Feature|Fix|Cleanup' | ||
|
|
||
| - name: Check 'Type:' | ||
| if: (failure() || success()) && (steps.type-match.outputs.match == '') | ||
| run: exit 1 | ||
|
|
||
| # Check PR description for 'Test Plan:' field. | ||
| - name: Match 'Test Plan:' | ||
| uses: actions-ecosystem/action-regex-match@v2 | ||
| id: test-plan-match | ||
| if: failure() || success() | ||
| with: | ||
| text: ${{ github.event.pull_request.body }} | ||
| regex: 'Test Plan:\s*[\s\S]*?' | ||
|
|
||
| - name: Check 'Test Plan:' | ||
| if: (failure() || success()) && (steps.test-plan-match.outputs.match == '') | ||
| run: exit 1 | ||
| text: ${{ github.event.pull_request.title }} | ||
| regex: ^[A-Z][A-Z0-9]+-\d+: .{1,64}$ | ||
|
|
|
Coverage statistics of your commit: |
|
Pull request title must follow the pattern: Pull request description must follow the Commit message format for RDK-E:
< JIRA TICKET >: < one line summary of change less than 65 characters > |
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.
Suppressed comments (1)
.github/workflows/commit_message_format.yml:63
- The workflow matches the PR title against the required pattern, but never fails the job if it doesn't match. As a result, an invalid PR title would still pass this workflow. Add a follow-up step that exits non-zero when
steps.pr-title-match.outputs.matchis empty (similar to the PR body check).
- name: Match PR title
uses: actions-ecosystem/action-regex-match@v2
id: pr-title-match
if: failure() || success()
with:
| with: | ||
| text: ${{ github.event.pull_request.body }} | ||
| regex: 'Jira:\s*[\s\S]*?' | ||
| regex: '(?s)^([A-Z][A-Z0-9]+-\d+): (.{1,64})\R\RReason for change:.*?(?:\RTest Procedure:.*)?$' | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/commit_message_format.yml:74
- The PR body regex makes the "Test Procedure:" section optional and also allows an empty "Reason for change:" section, but the workflow’s own guidance text says both fields are required. This means PRs can pass validation without providing required information.
regex: '^([A-Z][A-Z0-9]+-\d+): (.{1,64})\r?\n\r?\nReason for change:([\s\S]*?)(?:\r?\nTest Procedure:([\s\S]*))?$'
.github/workflows/commit_message_format.yml:65
- The PR title regex allows a summary that is only whitespace (e.g., "ABC-123: "), which violates the stated "one line summary" requirement. Consider requiring a non-whitespace character after the colon+space.
regex: '^[A-Z][A-Z0-9]+-\d+: .{1,64}$'
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/commit_message_format.yml:75
- The PR body regex makes the "Test Procedure:" section optional and also allows an empty/whitespace-only "Reason for change" (e.g., a body ending at
Reason for change:would still match). This weakens the validator compared to the format described in the workflow comment (items 2 and 3). Consider requiring both sections to be present and non-empty, and also avoid(.{1,64})so a\rfrom CRLF can be counted as part of the one-line summary.
with:
text: ${{ github.event.pull_request.body }}
regex: '^([A-Z][A-Z0-9]+-\d+): (.{1,64})\r?\n\r?\nReason for change:([\s\S]*?)(?:\r?\nTest Procedure:([\s\S]*))?$'
.github/workflows/commit_message_format.yml:56
- The example "Test Procedure" line includes a leading space inside the angle brackets and a placeholder with spaces ("JIRA TICKET"), which makes the example URL look invalid. Tightening this helps reduce confusion for contributors.
Test Procedure: < https://ccp.sys.comcast.net/browse/JIRA TICKET/url/to/test_step_section>
.github/workflows/commit_message_format.yml:76
- Step name is misleading: this step validates the PR body format, not the commit message. Renaming it will make workflow logs easier to understand when it fails.
- name: Check Commit message
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/commit_message_format.yml:66
- PR title validation isn't enforced: the regex match step always succeeds and only sets outputs, but there's no follow-up step that fails the job when
pr-title-match.outputs.matchis empty.
- name: Match PR title
uses: actions-ecosystem/action-regex-match@v2
id: pr-title-match
if: failure() || success()
with:
text: ${{ github.event.pull_request.title }}
regex: '^[A-Z][A-Z0-9]+-\d+: .{1,64}$'
.github/workflows/commit_message_format.yml:75
- The PR body regex makes
Test Procedure:optional and allows an emptyReason for change:value, but the workflow message describes both as required fields. This will let incomplete PR descriptions pass validation.
with:
text: ${{ github.event.pull_request.body }}
regex: '^([A-Z][A-Z0-9]+-\d+): (.{1,64})\r?\n\r?\nReason for change:([\s\S]*?)(?:\r?\nTest Procedure:([\s\S]*))?$'
.github/workflows/commit_message_format.yml:76
- Step name is misleading: it validates the PR body format, not a commit message. Renaming avoids confusion when troubleshooting workflow failures.
- name: Check Commit message
|
Coverage statistics of your commit: |
RDKEMW-22790: Commit message validator change
Reason for change: Change the commit message validator to follow the official Commit Message Format For RDKE
Test Procedure: Rialto CI