Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support BREAKING CHANGE #168

Merged
merged 8 commits into from
Feb 17, 2024
Merged

feat: support BREAKING CHANGE #168

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
53951bd
feat: support BREAKING CHANGE
hyoban Feb 15, 2024
f960103
refactor: use notes information
hyoban Feb 15, 2024
6a414fd
refactor: use parser
hyoban Feb 15, 2024
573c841
fix: regex
hyoban Feb 15, 2024
df62789
chore: clean
hyoban Feb 15, 2024
9b74215
docs: update README.md
hyoban Feb 17, 2024
d5a9dda
refactor: replace `@ts-expect-error`
hyoban Feb 17, 2024
9a3bd69
test: more
hyoban Feb 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ This is a release commit. Returning false.
Based on a commit's conventional commit message type:

1. If the type is `feat` `fix`, or `perf`, it's considered "meaningful"
2. If the type is `docs`, `refactor`, `style`, or `test`, it's ignored
3. If the message looks like `v1.2.3`, `chore: release 1.2.3`, or similar, it's considered a "release"
1. If the commit is marked as being a breaking change, either via a note or via an `!` appended to the type, it's considered "meaningful"
hyoban marked this conversation as resolved.
Show resolved Hide resolved
1. If the type is `docs`, `refactor`, `style`, or `test`, it's ignored
1. If the message looks like `v1.2.3`, `chore: release 1.2.3`, or similar, it's considered a "release"

See [`getCommitMeaning`](./src/getCommitMeaning.ts) for the exact logic used.

Expand Down
21 changes: 21 additions & 0 deletions src/getCommitMeaning.test.ts
View file
Open in desktop
hyoban marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,27 @@ describe("getCommitMeaning", () => {
["chore(deps): release", "release"],
["chore(deps): release 1.2.3", "release"],
["chore(deps): release v1.2.3", "release"],
["chore!: message", "meaningful"],
["docs!: message", "meaningful"],
["chore!: release", "meaningful"],
["feat!: a feature with a breaking change", "meaningful"],
["chore: bump\n\nBREAKING CHANGE: breaks things", "meaningful"],
["chore: bump\n\nBREAKING-CHANGE: breaks things", "meaningful"],
["docs: bump\n\nBREAKING CHANGE: breaks things", "meaningful"],
["docs: bump\n\nBREAKING-CHANGE: breaks things", "meaningful"],
[
"chore: deps\n\nBREAKING-CHANGE: breaks things\nMultiple: footer notes",
"meaningful",
],
["chore: deps\n\n! in the commit body", { type: "chore" }],
[
"chore: deps\n\nFooter-note: other than BREAKING CHANGE",
{ type: "chore" },
],
[
"chore: deps\n\nMultiple: footer notes\nMultiple: footer notes",
{ type: "chore" },
],
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
])("returns %j for %s", (input, expected) => {
expect(getCommitMeaning(input)).toEqual(expected);
});
Expand Down
9 changes: 8 additions & 1 deletion src/getCommitMeaning.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,14 @@ const releaseCommitTester =

export function getCommitMeaning(message: string) {
// Some types are always meaningful or ignored, regardless of potentially release-like messages
const { type } = conventionalCommitsParser.sync(message);
const { notes, type } = conventionalCommitsParser.sync(message, {
// @ts-expect-error - options from https://github.com/conventional-changelog/conventional-changelog/issues/648#issuecomment-704867077
breakingHeaderPattern: /^(\w*)(?:\((.*)\))?!: (.*)$/,
});
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Refactor] No need for a // @ts-expect-error when an assertion can do:

Suggested change
const { notes, type } = conventionalCommitsParser.sync(message, {
// @ts-expect-error - options from https://github.com/conventional-changelog/conventional-changelog/issues/648#issuecomment-704867077
breakingHeaderPattern: /^(\w*)(?:\((.*)\))?!: (.*)$/,
});
// options from https://github.com/conventional-changelog/conventional-changelog/issues/648#issuecomment-704867077
const { notes, type } = conventionalCommitsParser.sync(message, {
breakingHeaderPattern: /^(\w*)(?:\((.*)\))?!: (.*)$/,
} as object);

This is actually a great opportunity to clean up the DefinitelyTyped types for conventional-commits-parser. Is that something you have time for too? No worries if not, I will if you don't. 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to deal with it. It’s always exciting to have the opportunity to work on an open source project.

if (notes.some((note) => note.title.match(/^BREAKING[ -]CHANGE$/))) {
return "meaningful";
}

if (type) {
if (alwaysMeaningfulTypes.has(type)) {
return "meaningful";
Expand Down