🔧 fix: allow scoped PR titles#31
Conversation
|
| Filename | Overview |
|---|---|
| ghcontrib/pr_title.go | New file introducing regex-based PR title validation with support for optional scope and breaking marker; logic is correct and well-structured |
| ghcontrib/pr_test.go | Comprehensive test suite for valid and invalid PR title forms including scoped, breaking, colon-alias, and mixed combinations |
| ghcontrib/pr.go | Removes old prefix-based validation in favour of the new regex-backed validatePullRequestTitle; clean refactor with no logic gaps |
| ghrelease/create-release.sh | Introduces commit_type() helper to normalise emoji/colon-alias prefixes before classification; refactor commits always force a major bump; refactor!: lands in Refactors rather than Breaking Changes |
| ghrelease/main.go | Doc-comment update to reflect scoped commit classification; no functional changes |
| ghrelease/README.md | New table documents classification rules but omits the scoped-breaking forms and the refactor! edge case |
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
ghrelease/README.md:15
The Breaking Changes row only lists the unscoped `!` forms, but the script also routes `feat(scope)!:`, `fix(scope)!:`, and `chore(scope)!:` to the Breaking Changes section. Additionally, `refactor!:` and `refactor(scope)!:` are routed to the Refactors section (not Breaking Changes) — a reader using the `!` marker on a refactor commit would not find it in the Breaking Changes changelog entry. Both gaps are worth surfacing here.
```suggestion
| `feat!: ...`, `feat(scope)!: ...`, `fix!: ...`, `fix(scope)!: ...`, `chore!: ...`, or `chore(scope)!: ...` | Major bump, Breaking Changes section |
| `refactor!: ...` or `refactor(scope)!: ...` | Major bump, Refactors section (not Breaking Changes) |
```
### Issue 2 of 2
ghrelease/create-release.sh:51
`refactor!:` treated the same as `refactor:` — both land in Refactors, not Breaking Changes. The PR title checker permits `♻️ refactor!:` (the `!?` in the regex makes the breaking marker valid), so a contributor who writes `♻️ refactor!: remove public API` will expect that commit to appear under Breaking Changes in the release notes, not Refactors. If this is intentional, a comment here explaining why `!` is ignored for the refactor type would prevent future confusion.
Reviews (3): Last reviewed commit: "🔧 fix(ghcontrib): allow scoped PR title..." | Re-trigger Greptile
f35da17 to
d647d91
Compare
jpmcb
left a comment
There was a problem hiding this comment.
This is great! The only thing I'm unsure of: refactors being major bumps - is the idea here that, with a refactor, we're guarding on SDK / API changes and expecting a breaking change?
d647d91 to
9f40734
Compare
|
Yeah, I was a little to fast and loose with the refactor. I've moved it down to patch by default which was what I had in my head and not what came out in the PR |
Summary
🔧 fix(percolator): ....:wrench: fix(scope): ....♻️ refactorrelease commits patch-level by default while lettingrefactor!opt into a major bump.How it works
The PR title checker now builds the allowed emoji/type pairs into regexes that accept an optional lowercase scope and optional breaking marker before the colon.
The release script normalizes the supported emoji or colon-alias prefix before classifying
feat,fix, andchore, sofix(scope):is treated the same asfix:for patch releases.Refactor commits are classified separately into a Refactors release-note section. Plain refactors stay patch-level under the current release model;
refactor!andrefactor(scope)!are treated as breaking changes and force a major bump.Test plan
direnv exec . go test ./pr_title.go ./pr_test.gosh -n ghrelease/create-release.shcheck-pull-requestagainst paper PR #52Fixes PCC-615