feat(lint-pr-title): add action to validate PR titles - #82
Conversation
Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
Summary by CodeRabbit
WalkthroughChangesPR title linting
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ec02e67-efb0-42c7-90d8-e755b8f898cf
📒 Files selected for processing (4)
lint-pr-title/PRTitle.ps1lint-pr-title/README.mdlint-pr-title/action.ymltests/PRTitle.Tests.ps1
| - name: Check PR Title | ||
| uses: ScoopInstaller/GithubActions/lint-pr-title@main |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
git ls-remote https://github.com/ScoopInstaller/GithubActions.git refs/heads/mainRepository: ScoopInstaller/GithubActions
Length of output: 223
Pin the documented action reference to a full-length commit SHA.
@main can change after the README is published, so consumers need to review additional action changes. Use the current full SHA, e.g. uses: ScoopInstaller/GithubActions/lint-pr-title@e174c3bef2aeec16a40f2f075cafa167733f0a3e, and update it when the action pin is intentionally upgraded.
There was a problem hiding this comment.
@coderabbitai Nope, we haven't merged this PR and there's no full SHA for it.
Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a new composable composite action (lint-pr-title/) that validates pull request titles against the Scoop bucket PR title convention described in Issue #81, along with PowerShell implementation and Pester tests.
Changes:
- Added a composite GitHub Action to validate PR titles on
pull_requestevents. - Implemented
Test-PRTitlePowerShell validation logic and documented expected formats/rules. - Added Pester tests to exercise valid/invalid PR title cases.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
lint-pr-title/action.yml |
Composite action entrypoint that reads PR title and fails the workflow on invalid titles. |
lint-pr-title/PRTitle.ps1 |
PowerShell regex-based PR title validator function (Test-PRTitle). |
lint-pr-title/README.md |
Usage docs and rule summary for downstream consumers of the action. |
tests/PRTitle.Tests.ps1 |
Pester test suite covering valid/invalid PR title examples. |
Comments suppressed due to low confidence (1)
tests/PRTitle.Tests.ps1:37
- This test currently asserts that a manifest name containing a dot ("app.name") is valid, but Issue #81 calls out manifest names as lowercase letters/numbers/hyphens only. This should be an invalid-title test instead (or removed).
It 'Should accept manifest with dot in name' {
Test-PRTitle 'app.name@1.0: update' | Should -BeTrue
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (3)
lint-pr-title/PRTitle.ps1:17
- The regex currently accepts titles where the description is only whitespace (e.g.
app-name:), because the tail: .+matches spaces. It also allows)inside the@versionsegment (@[^^\s:(]+), which can accept malformed titles likeapp@1.0): .... Tighten the pattern to require a non-whitespace description and exclude both parentheses from the version token.
if ([string]::IsNullOrEmpty($Title)) { return $false }
$re = '^(\(chore\)|[a-z0-9]([a-z0-9.-]*[a-z0-9-])?(\(\*\)|\((?=[a-z0-9.-]*[a-z0-9])[a-z0-9.-]*[a-z0-9-]\)|@[^\s:(]+)?): .+'
return $Title -cmatch $re
tests/PRTitle.Tests.ps1:103
- There’s no test covering the case where the title has a colon but the description is whitespace-only (e.g.
app-name:). Given the intent is: <description>, this should be rejected and covered by a test so regressions are caught.
}
It 'Should reject missing description' {
Test-PRTitle 'app-name: ' | Should -BeFalse
}
lint-pr-title/README.md:41
- The README states the New manifest format must be
Add version <version>, but the action implementation only validates the general shape<manifest-name>: <description>and does not enforce a specific phrase. Update the documentation to match the actual validation rules (or update the validator to enforce this rule consistently).
- **New manifest:** `<manifest-name>: Add version <version>`
- **Manifest update:** `<manifest-name>@<version>: <description>`
- **Multi-manifest:** `<manifest-name>(*): <description>` or `<manifest-name>(<suffix>): <description>`
- **Maintenance:** `(chore): <description>`
|
This PR evolves manifest PR title linting from a convention-based prompt into a formal constraint, as part of the effort to alleviate the maintenance burden on buckets. I will merge it if there are no further reviews, or approval from another maintainer. Then promote it to official buckets later after #84. @ScoopInstaller/maintainers |
Part of #81
Example:
AI-assisted