commitlint icon indicating copy to clipboard operation
commitlint copied to clipboard

"footer-empty" rule does not recognize footer

Open ND56 opened this issue 3 years ago • 5 comments

I'm trying to use the footer-empty rule fined in the documentation but it seems to be erroneously flagging my footer as not present.

Expected Behavior

This is my configuration:

const Configuration = {
  /*
   * Resolve and load @commitlint/config-conventional from node_modules.
   * Referenced packages must be installed
   */
  extends: ["@commitlint/config-conventional"],
  /*
   * Any rules defined here will override rules from @commitlint/config-conventional
   */
  rules: {
    "footer-empty": [2, "never"],
  },
};

module.exports = Configuration;

As a result, I expected this commit to work:

chore: testing footer lint

footer: I expect this to work
# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# On branch add_commit_linter
# Changes to be committed:
#       modified:   commitlint.config.js
#

Current Behavior

Currently, I get this error when I commit the code:

commitlint...............................................................Failed
- hook id: commitlint
- duration: 0.39s
- exit code: 1

â§—   input: chore: testing footer lint

footer: I expect this to work
✖   footer may not be empty [footer-empty]
✖   found 1 problems, 0 warnings 
    (Need help? -> https://github.com/conventional-changelog/commitlint#what-is-commitlint )

My Environment

Executable Version
commitlint --version @commitlint/cli": "^16.2.1"
git --version git version 2.32.0 (Apple Git-132)
node --version Node.js v17.5.0

ND56 avatar Feb 17 '22 00:02 ND56

Thanks, this seems to be broken.

Footer rule code is here.

I could be wrong but it also looks like as if the test is wrong in the first place?
filled is this:

filled: 'test: subject\nBREAKING CHANGE: something important',

But I would think it should more look like this?:

filled: 'test: subject\nBREAKING CHANGE: something important\nA footer might be here',

This seems to be wrong in all footer related tests. Either I'm reading it wrong or it was broken form the beginning?
BREAKING CHANGE: is a body, no? If so this might be related: #896

Would you have time and motivation for a PR?

escapedcat avatar Feb 19 '22 06:02 escapedcat

@escapedcat I'm not an expert on conventional-commit conventions, but my read of of the docs suggests that a "BREAKING CHANGE" line would actually be used as a footer; e.g., see the docs here.

ND56 avatar Feb 21 '22 21:02 ND56

Thanks! You're right. In this case our tests make sense which is a relief :P

In this case I'm wondering how the current code is parsing the footer or recognises a footer.
BREAKING CHANGE seems to work fine with your config.

escapedcat avatar Feb 22 '22 07:02 escapedcat

Also noticed the same behaviour when I allow empty body, but not allow empty footer. Then:

test(general): some change

Issue: #1

fails, because Issue: #1 seems to be treated as body.

jaklan avatar Apr 19 '22 08:04 jaklan

Hmm footer not parsed if we use yarn commitlint -g ./commitlint.config.js --edit $1 line in file commit-msg. But it works if we use cat $1 | yarn commitlint -g ./commitlint.config.js instead. Maybe it's help to dig it.

bimawa avatar Apr 25 '22 03:04 bimawa