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.

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

"footer-empty" rule does not recognize footer #3034

Open
ND56 opened this issue Feb 17, 2022 · 5 comments
Open

"footer-empty" rule does not recognize footer #3034

ND56 opened this issue Feb 17, 2022 · 5 comments
Labels

Comments

@ND56
Copy link

ND56 commented Feb 17, 2022

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
@escapedcat
Copy link
Member

escapedcat commented Feb 19, 2022

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 escapedcat added the bug label Feb 19, 2022
@ND56
Copy link
Author

ND56 commented Feb 21, 2022

@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.

@escapedcat
Copy link
Member

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.

@jaklan
Copy link

jaklan commented Apr 19, 2022

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.

@bimawa
Copy link

bimawa commented Apr 25, 2022

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.

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

No branches or pull requests

4 participants