Skip to content

Accept revert commit titles that are elongated by git #97

Description

@RaisinTen

In nodejs/node#42934, I was trying to revert nodejs/node#27371, so git revert automatically created this commit title for me:

Revert "bootstrap: delay the instantiation of maps in per-context scripts"

which has 74 characters. Since it exceeds our maximum limit of 72 characters, the commit message linter failed on CI - https://github.com/nodejs/node/runs/6248053065?check_suite_focus=true#step:6:15

Details
Run echo "::add-matcher::.github/workflows/commit-lint-problem-matcher.json"
npm WARN exec The following package was not found and will be installed: core-validate-commit
# b04fe688d5859f707cf1a5e0206967268118bf7a
ok 1 co-authored-by-is-trailer: no Co-authored-by metadata
ok 2 fixes-url: skipping fixes-url # SKIP
ok 3 line-after-title: blank line after title
ok 4 line-length: line-lengths are valid
ok 5 subsystem: valid subsystems [bootstrap]
ok 6 title-format: Title is formatted correctly.
Error: not ok 7 title-length: Title must be <= 72 columns.
  ---
    {
      found: 74,
      compare: '<=',
      wanted: 72,
      at: {
        line: 0,
        column: 72,
        body: 'Revert "bootstrap: delay the instantiation of maps in per-context scripts"'
      }
    }
  ...

0..7
# tests 7
# pass  5
# fail  1
# Please review the commit message guidelines:
# https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

As a workaround, I had to change the commit title to

bootstrap: stop delaying instantiation of maps in per-context scripts

which has 69 characters (< 72) but it isn't satisfying anymore. :/

Instead of failing, IMO the linter should accept standard revert commit titles regardless of the size.

Activity

  1. tniessen commented on Jun 2, 2022

    @tniessen
    Member

    I think we used to restrict the title to 50 chars, which is conventional and works well with other tools, but now that the limit is softer, this can happen. So I guess the only solution is indeed to also relax this limit, but probably only if it matches /^Revert ".*"$/.

  2. RaisinTen commented on Jun 26, 2022

    @RaisinTen
    MemberAuthor

    Agreed, sent a PR to implement this - #99

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions