Add --include-messages option to filter commits by subject regex#76
Add --include-messages option to filter commits by subject regex#76autumn-n wants to merge 1 commit into
Conversation
axelniklasson
left a comment
There was a problem hiding this comment.
Thanks for taking the time to raise this pull request @autumn-n! This change makes sense to me, but there are some things we need to address before landing this PR.
Apart from the code comments I think we should also instead call the argument --include-subjects as we're only matching on the subject and not the full commit message.
| includePaths: string[] | null, | ||
| includeMessages: string | null, |
There was a problem hiding this comment.
Let's unify includePaths and includeMessages (but renamed to includeSubjects) into an options object. We can call it ScanOptions:
type ScanOptions = {
includePaths?: string[] | null;
includeSubjects?: string | null;
}
|
|
||
| for (const commit of commits) { | ||
| if (messageRegex) { | ||
| const subject = (commit.message ?? "").split("\n", 1)[0]!; |
There was a problem hiding this comment.
Let's extract out the subject parsing we already have in https://github.com/linear/linear-release/blob/main/src/extractors.ts#L185 to a helper and use that one here. We should not do any non-null assertions here.
| const subject = (commit.message ?? "").split("\n", 1)[0]!; | ||
| if (!messageRegex.test(subject)) { | ||
| verbose(`Skipping commit ${commit.sha} — subject does not match --include-messages`); | ||
| continue; |
There was a problem hiding this comment.
Skipping the commit here means revert commits never reach extractRevertedIssueIdentifiersForCommit(), so filtered runs can leave reverted issues attached. We should apply the regex to the effective subject for revert commits, not the raw outer subject. For example, if the subject is Revert "fix: login bug. Fixes ENG-100", we should test fix: login bug. Fixes ENG-100, not the full Revert "fix: login bug. Fixes ENG-100" wrapper.
The scanner is doing last-write-wins over the commit range:
- fix: login bug. Fixes ENG-100
- Revert "fix: login bug. Fixes ENG-100"
With --include-messages="^(feat|fix|perf):":
- commit 1 matches, so ENG-100 is recorded as added
- commit 2 does not match, because its subject starts with Revert "..."
- the code continues before extractRevertedIssueIdentifiersForCommit() runs
- so nothing ever flips ENG-100 from added to reverted
There was a problem hiding this comment.
Left some comments. The anchored comments from @axel cover the critical issues (revert-commit bypass, non-null assertions, ScanOptions refactoring) and must be addressed before merging. I found one additional gap: invalid regex patterns from user input are not validated at arg-parse time, which will crash with an unhandled SyntaxError mid-operation.
Adds
--include-messages, a regex filter on the commit subject. Used alongside--include-paths, it lets you drop commits that shouldn't show up in a release — bot pushes, direct commits without an issue link, anything that doesn't fit your team's conventions — without rewriting history.The regex matches the subject only (everything before the first
\n) so squash dumps and trailers in the body can't accidentally match. The pattern is stored as a string and compiled once insidescanCommits, mirroring how--include-pathsis stored as a string array and turned into pathspec arguments at the point of use. Both filters compose: a commit must pass both to be scanned.Examples