dependabot / dependabot-core

🤖 Dependabot's core logic for creating update PRs.
https://docs.github.com/en/code-security/dependabot
MIT License
4.66k stars 1.01k forks source link

commitlint: body's lines must not be longer than 100 characters [body-max-line-length] #2445

Open felipecrs opened 4 years ago

felipecrs commented 4 years ago

Dependabot commit messages body line lengths do not conform with commitlint's defaults.

Take a look at https://github.com/felipecassiors/megatar/pull/13 and https://github.com/felipecassiors/megatar/pull/14

It fails with:

⧗   input: build(deps-dev): bump @typescript-eslint/eslint-plugin

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 3.8.0 to 3.9.0.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v3.9.0/packages/eslint-plugin)

Signed-off-by: dependabot[bot] <support@github.com>
✖   body's lines must not be longer than 100 characters [body-max-line-length]

✖   found 1 problems, 0 warnings
ⓘ   Get help: https://www.conventionalcommits.org/

https://github.com/felipecassiors/megatar/pull/13/checks?check_run_id=969926394#step:4:19

ttshivers commented 4 years ago

This goes along with a similar issue #2056, where Dependabot will also make the commit header too long

caugner commented 2 years ago

I don't think this is a bug in Dependabot, but rather it should be easier to make commitlint ignore Dependabot's commits. One solution would be to add an ignores option to the header-max-length rule which accepts one or more regular expressions.

felipecrs commented 2 years ago

Or perhaps it should be smarter when URLs are detected. There is no way to break an URL into multiple lines.

caugner commented 2 years ago

Just found out via https://stackoverflow.com/a/60195181 that commitlint indeed already has an ignores option:

  /*
   * Functions that return true if commitlint should ignore the given message.
   */
  ignores: [(commit) => commit === ''],

So you can make commitlint ignore Dependabot's commits as follows:

// commitlint.config.js
module.exports = {
  extends: ['@commitlint/config-conventional'],
  ignores: [(message) => /^Bumps \[.+]\(.+\) from .+ to .+\.$/m.test(message)],
}
jurre commented 2 years ago

I think this is covered by https://github.com/dependabot/dependabot-core/issues/1666, so I'm going to close this as a duplicate, feel free to add any context you feel is missing from that issue in the comments there.

felipecrs commented 2 years ago

@jurre it's not the same thing, this is about body line while the one you refer is about the subject line.

jurre commented 2 years ago

Meant to link to https://github.com/dependabot/dependabot-core/issues/1666

jeffwidman commented 1 year ago

Re-opened per:

keikoro commented 2 days ago

Or perhaps it should be smarter when URLs are detected. There is no way to break an URL into multiple lines.

@felipecrs I don't know how much of a difference this would make for grouped updates. I removed the URLs from the first line of an update to a group of four dependencies and it's still too long.

Interestingly, in a PR which would update five dependencies at once, they are put in a Markdown table instead. Which would work with the URLs removed. Though that PR exceeds the allowed subject length. :upside_down_face:

I feel like an on/off switch for the inclusion of project URLs for dependencies in Dependabot messages might be the most useful solution. Or, better yet, a differentiation between Git commit messages and PR (meta) info. That Markdown table for grouped dependencies isn't useful outside of Markdown-based UIs, and neither are Markdown-formatted URLs.

Edit: A combo solution could be a config option for commit message verbosity – "minimal, plain text" (without URLs) vs. "verbose" (Markdown-formatted, with URLs).