Skip to content

Conversation

adamchristiansen
Copy link

Description

A breaking change can be denoted by either ! in the header (e.g., feat!: add advanced search filters) or by including BREAKING CHANGE in the footer (e.g., BREAKING CHANGE: old queries no longer work with the new API.). This pull request adds the breaking-change-exclamation-mark rule that requires a BREAKING CHANGE footer if ! appears in the header and a ! in the header if BREAKING CHANGE appears in the footer. The idea is that it is all or nothing, so if one breaking change marker is present the other must be present too.

Motivation and Context

Implements and closes #4547.

This feature enforces a solution to three problems:

  1. Using just ! might not convey enough information. For example, feat!: add advanced search filters. Why is this a breaking change? How do I know what broke unless the footer is present? Using this rule will fail because a footer indicating a breaking change must be added in this case.
  2. Using just BREAKING CHANGE might cause confusion. For example, if I use something like git log --online to quickly look at changes and I see feat: add advanced search filters, I have no idea that it is a breaking change without looking at that specific commit in greater detail. This rule will fail because an exclamation mark must be added to the head in this case.
  3. This rule enforces the broadest tooling coverage since both means of denoting breaking changes are present in commit messages.

Usage examples

// commitlint.config.js
module.exports = {
  rules: {
    'breaking-change-exclamation-mark': [2, 'always']
  }
};
echo "feat!: add something\n\nBREAKING CHANGE: breaks something" | commitlint # passes
echo "feat: add something" | commitlint # passes
echo "feat!: add something" | commitlint # fails
echo "feat: add something\n\nBREAKING CHANGE: breaks something" | commitlint # fails

How Has This Been Tested?

I added a new test @commitlint/rules/src/breaking-change-exclamation-mark.test.ts that follows the structure of existing tests. The commit messages to tests were decided based on equivalence partitions.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

codesandbox-ci bot commented Sep 24, 2025

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@escapedcat escapedcat requested a review from JounQin September 24, 2025 08:17
return [true];
}

const hasExclamationMark = !!header && /!:/.test(header);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

I will make this change.

Should this be updated to use the breakingHeaderPattern too? To me, that is outside of the scope of this pull request and should be a separate commit.

const hasExclamationMark = /!:/.test(input);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Personally I think such tiny change is fine to be inclined in one PR.

Copy link
Author

Choose a reason for hiding this comment

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

I will make the change.

}

const hasExclamationMark = !!header && /!:/.test(header);
const hasBreakingChange = !!footer && /BREAKING[ -]CHANGE:/.test(footer);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need make sure the footer line starts with BREAKING not only includes?

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. I will modify the regex for a multiline search.

import message from "@commitlint/message";
import { SyncRule } from "@commitlint/types";

export const breakingChangeExclamationMark: SyncRule = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

The new rule should be added into

export type RulesConfig<V = RuleConfigQuality.User> = {

adamchristiansen added a commit to adamchristiansen/commitlint that referenced this pull request Sep 25, 2025
Addresses review feedback from @JounQin about pull request conventional-changelog#4548.

- Use `breakingHeaderPattern` to search for the exclamation mark in the header.
- Correct the regular expression to require that BREAKING CHANGE in the footer be anchored at the
  beginning of a line.
- Updated the `RulesConfig` type.
import { typeMinLength } from "./type-min-length.js";

export default {
"breaking-change-exclamation-mark": breakingChangeExclamationMark,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we resort them alphabetically?

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. I will fix the sorting.

@@ -1,5 +1,19 @@
# Rules

## breaking-change-exclamation-mark
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above about the order.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed again; I will fix this. It has bothered me in the past looking things up on this page as it is about 90% in the right order.

@adamchristiansen adamchristiansen force-pushed the master branch 3 times, most recently from 4ba9f06 to 28e06e1 Compare September 26, 2025 04:48
Addresses review feedback from @JounQin about pull request conventional-changelog#4548.

- Use `breakingHeaderPattern` to search for the exclamation mark in the header.
- Correct the regular expression to require that BREAKING CHANGE in the footer be anchored at the
  beginning of a line.
- Updated the `RulesConfig` type.
Address pull request feedback from @JounQin.

- Fixed regex for `subject-exclamation-mark`.
- Fixed sorting for `RulesConfig`.
- Fixed sorting for `rules.md`.
@adamchristiansen
Copy link
Author

I made several pushes because I resolved a conflict with a merge instead of performing a rebase 🤦. I have implemented all suggestions and rebased the branch onto the latest master, so I believe it is ready now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

feat: rule to require both ! and BREAKING CHANGE
2 participants