docs: add instructions for pre-commit #2715

pkoch · 2021-08-11T14:04:38Z

Add support for pre-commit.

Description

Added a configuration that works for me to the docs.

Motivation and Context

I wanted to use conventional-changelog/commitlint with pre-commit.

Usage examples

Doesn't apply, it's a doc change.

How Has This Been Tested?

Used this file as a .pre-commit-config.yaml:

repos:
    - repo: local
      hooks:
        - id: commitlint
          name: commitlint
          stages: [commit-msg]
          language: node
          additional_dependencies:
            - "commitlint"
            - "@commitlint/config-conventional"
          entry: commitlint --edit

And tried it out:

gato:test-alessandrojcm pkoch$ pre-commit install -t commit-msg --install-hooks
gato:test-alessandrojcm pkoch$ git ci --allow-empty -am "derp"
commitlint...............................................................Failed
- hook id: commitlint
- exit code: 1

⧗   input: derp
✖   subject may not be empty [subject-empty]
✖   type may not be empty [type-empty]

✖   found 2 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

gato:test-alessandrojcm pkoch$ git ci --allow-empty -am "docs: demo"
commitlint...............................................................Passed
[master 1083c95] docs: demo

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)

Non of the above. It's a doc only 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.

escapedcat · 2021-08-12T01:51:31Z

Hey @pkoch , thanks for your PR!

You can add documentation here:

AdeAttwood · 2021-08-12T06:47:58Z

@pkoch can I be a pain and ask you to put it under a new main header "Integrations". This will help me out for a couple of features on the roadmap

Example of the new header from

https://github.com/conventional-changelog/commitlint/blob/master/README.md#contents

...

Tools
Version Support and Releaes
Integrations
- Pre Commit <-- here
Related projects
License
Development
- Install and run
- Publishing a release

pkoch · 2021-08-20T11:31:35Z

Sorry for the radio silence, folks. I'll document this better soon.

I'm having a problem with pre-commit, where it tries to install conventional-changelog/commitlint, with both prod and dev, and fails.

pkoch · 2021-08-21T11:55:21Z

Here's the output:

[INFO] Initializing environment for [email protected]:pkoch/commitlint.git.
[INFO] Installing environment for [email protected]:pkoch/commitlint.git.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/Users/pkoch/.cache/pre-commit/repoxofddv9q/node_env-system/bin/node', '/usr/local/bin/npm', 'install', '--dev', '--prod', '--ignore-prepublish', '--no-progress', '--no-save')
return code: 1
expected return code: 0
stdout: (none)
stderr:
    npm WARN install Usage of the `--dev` option is deprecated. Use `--include=dev` instead.
    npm notice 
    npm notice New minor version of npm available! 7.6.0 -> 7.21.0
    npm notice Changelog: <https://github.com/npm/cli/releases/tag/v7.21.0>
    npm notice Run `npm install -g [email protected]` to update!
    npm notice 
    npm ERR! code ERESOLVE
    npm ERR! ERESOLVE unable to resolve dependency tree
    npm ERR! 
    npm ERR! Found: [email protected]
    npm ERR! node_modules/inquirer
    npm ERR!   inquirer@"^7.3.3" from @lerna/[email protected]
    npm ERR!   node_modules/@lerna/prompt
    npm ERR!     @lerna/prompt@"4.0.0" from @lerna/[email protected]
    npm ERR!     node_modules/@lerna/clean
    npm ERR!       @lerna/clean@"4.0.0" from [email protected]
    npm ERR!       node_modules/lerna
    npm ERR!         dev lerna@"^4.0.0" from the root project
    npm ERR!         1 more (@commitlint/config-lerna-scopes)
    npm ERR!     @lerna/prompt@"4.0.0" from @lerna/[email protected]
    npm ERR!     node_modules/@lerna/import
    npm ERR!       @lerna/import@"4.0.0" from [email protected]
    npm ERR!       node_modules/lerna
    npm ERR!         dev lerna@"^4.0.0" from the root project
    npm ERR!         1 more (@commitlint/config-lerna-scopes)
    npm ERR!     3 more (@lerna/publish, @lerna/version, @lerna/otplease)
    npm ERR! 
    npm ERR! Could not resolve dependency:
    npm ERR! peer inquirer@"^8.0.0" from @commitlint/[email protected]
    npm ERR! @commitlint/cz-commitlint
    npm ERR! 
    npm ERR! Fix the upstream dependency conflict, or retry
    npm ERR! this command with --force, or --legacy-peer-deps
    npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
    npm ERR! 
    npm ERR! See /Users/pkoch/.npm/eresolve-report.txt for a full report.
    
    npm ERR! A complete log of this run can be found in:
    npm ERR!     /Users/pkoch/.npm/_logs/2021-08-21T11_35_01_043Z-debug.log
    
Check the log at /Users/pkoch/.cache/pre-commit/pre-commit.log

pre-commit installs things with the line you can see on the output above (npm install --dev --prod --ignore-prepublish --no-progress --no-save). There's currently no way to configure this.

Because this project (commitlint) is declared as a runtime dependency on alessandrojcm/commitlint-pre-commit-hook, that makes it not want to install @lerna/[email protected] (with is a dev dependecy on this project), and that's why we only see this problem on this repo.

@lerna/[email protected] wants inquirer@"^7.3.3". @commitlint/[email protected] wants inquirer@"^8.0.0" as a peer. I don't know how to untie that knot. :x

pkoch · 2021-08-21T12:00:56Z

For my own reference, here's the .pre-commit-config.yaml file I've been using to test this:

repos:
    - repo: [email protected]:pkoch/commitlint.git
      rev: add-pre-commit-hook
      hooks:
          - id: commitlint

pkoch · 2021-08-21T12:21:51Z

I tried to force pre-commit to use node 14.17.5, but that version doesn't have darwin-arm64 builds, so I can't test it. :/

Mouvedia · 2021-08-22T13:34:02Z

just add legacy-peer-deps=true to .npmrc file

pkoch · 2021-08-22T21:18:41Z

Yep, that solved it! 🎉 But, a new challenger appears.

gato:test-alessandrojcm pkoch$ pre-commit install -t commit-msg --install-hooks
pre-commit installed at .git/hooks/commit-msg
[WARNING] The 'rev' field of repo 'https://github.com/pkoch/commitlint' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
[INFO] Initializing environment for https://github.com/pkoch/commitlint.
[INFO] Initializing environment for https://github.com/pkoch/commitlint:@commitlint/config-conventional.
[INFO] Installing environment for https://github.com/pkoch/commitlint.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('/Users/pkoch/.cache/pre-commit/repotccv81mr/node_env-system/bin/node', '/usr/local/bin/npm', 'install', '--dev', '--prod', '--ignore-prepublish', '--no-progress', '--no-save')
return code: 127
expected return code: 0
stdout:
    
    > @commitlint/[email protected] postinstall
    > yarn husky install
    
    
stderr:
    npm WARN install Usage of the `--dev` option is deprecated. Use `--include=dev` instead.
    npm WARN deprecated [email protected]: this library is no longer supported
    npm WARN deprecated [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
    npm WARN deprecated [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
    npm WARN deprecated [email protected]: core-js@<3.3 is no longer maintained and not recommended for usage due to the number of issues. Because of the V8 engine whims, feature detection in old core-js versions could cause a slowdown up to 100x even if nothing is polyfilled. Please, upgrade your dependencies to the actual version of core-js.
    sh: yarn: command not found
    npm ERR! code 127
    npm ERR! path /Users/pkoch/.cache/pre-commit/repotccv81mr
    npm ERR! command failed
    npm ERR! command sh -c yarn husky install
    
    npm ERR! A complete log of this run can be found in:
    npm ERR!     /Users/pkoch/.npm/_logs/2021-08-22T21_15_33_324Z-debug.log
    
Check the log at /Users/pkoch/.cache/pre-commit/pre-commit.log

This is getting weirder and weirder, all over being forced to install the dev dependencies. Maybe we should just continue to use @alessandrojcm's repo, or even ask pre-commit maintainers to add this to the mirrors they build?

pkoch · 2021-08-22T21:19:39Z

(we should still probably commit the .npmrc, nonetheless; that feels pretty unrelated)

pkoch · 2021-08-22T23:22:09Z

Replaced yarn with npm run or npx.

Got to this:

An unexpected error has occurred: CalledProcessError: command: ('/Users/pkoch/.cache/pre-commit/repo2lh3cd7y/node_env-system/bin/node', '/usr/local/bin/npm', 'install', '-g', '/Users/pkoch/.cache/pre-commit/repo2lh3cd7y/commitlint-root-1.0.0.tgz', '@commitlint/config-conventional')
return code: 1
expected return code: 0
stdout: (none)
stderr:
    npm ERR! code 1
    npm ERR! path /Users/pkoch/.cache/pre-commit/repo2lh3cd7y/node_env-system/lib/node_modules/@commitlint/root
    npm ERR! command failed
    npm ERR! command sh -c npx husky install
    npm ERR! .git can't be found (see https://git.io/Jc3F9)
    
    npm ERR! A complete log of this run can be found in:
    npm ERR!     /Users/pkoch/.npm/_logs/2021-08-22T23_15_23_544Z-debug.log
    
Check the log at /Users/pkoch/.cache/pre-commit/pre-commit.log

Which I got around with "postinstall": "cd ../../../../.. && npx husky install", which feels very wrong.

For reference, here's what pre-commit install procedure looks like: https://github.com/pre-commit/pre-commit/blob/509e4e20e86b5db48bc827753dcdfcdbbd706812/pre_commit/languages/node.py#L99

I feels like I'm taking non-trivial steps with this PR. Guidance would be appreciated.

pkoch · 2021-08-22T23:38:52Z

So, I kept banging with my head on this, and got to a working config that doesn't need anything on this repo:

repos:
    - repo: local
      hooks:
        - id: commitlint
          name: commitlint
          stages: [commit-msg]
          language: node
          additional_dependencies:
            - "commitlint"
            - "@commitlint/config-conventional"
          entry: commitlint --edit

Using a local hook feels a bit inelegant, but it's effective. Perhaps a doc-only change is actually enough?

pkoch · 2021-08-23T00:36:41Z

Updated the top-post to match the new change. Looking for reviews! :)

escapedcat · 2021-08-23T02:50:06Z

🎉 thanks for not giving up :)

pkoch · 2021-08-23T09:03:40Z

Sorry, I'm not ready to merge. Just realized that this doesn't update the versions. 🤦

pkoch · 2021-08-28T01:54:22Z

Waiting for pre-commit/pre-commit-mirror-maker#101 to get in, in order to provide a more pleasant experience.

I think the next step will be to convince you folks to have a separate repo following the style of https://github.com/pre-commit/mirrors-prettier. The secret sauce is that it'll autonomously update itself to follow the main repo. However, it'll be a non-dev dependency, so we'll side-step all the issues I ran into (like needing yarn).

Thank you all for your patience and help so far! :)

Kurt-von-Laven · 2021-09-22T05:23:00Z

@pkoch, thank you for the effort you have put into this. It's greatly appreciated. I see that the pull request you linked has been closed, so I'm curious if there is another upstream PR in progress or in general what work would be needed to support this hook?

pkoch · 2021-09-22T08:50:42Z

That PR still needs follow-up work, which I haven't been able to find the time for.

To unblock delivering value on this, the easiest way would be to use an external repo like https://github.com/alessandrojcm/commitlint-pre-commit-hook for now.

Maybe we should adapt the instructions in the README and merge it?

Kurt-von-Laven · 2021-09-22T09:09:47Z

I'm a big fan of iterative improvements in open-source especially. Doesn't stop anyone from making it better after all. I'm not affiliated with commitlint though, so that's just my personal opinion.

ssbarnea · 2021-10-15T10:21:45Z

Something is not ok with the current recommendation. I followed it and I still get some very weird behavior from pre-commit, basically it runs twice, once passes and once fails with an exception:

commitlint...............................................................Failed
- hook id: commitlint
- exit code: 1

⧗   input: xxx: remove husky
✖   type must be one of [build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test] [type-enum]

✖   found 1 problems, 0 warnings
ⓘ   Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint

commitlint...............................................................Failed
- hook id: commitlint
- exit code: 1

/Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/cli/lib/cli.js:112
        throw err;
        ^

Error: Cannot find module "@commitlint/config-conventional" from "/Users/ssbarnea/c/a/vscode-ansible"
    at resolveId (/Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/resolve-extends/lib/index.js:97:17)
    at resolveConfig (/Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/resolve-extends/lib/index.js:81:26)
    at /Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/resolve-extends/lib/index.js:41:26
    at Array.reduce (<anonymous>)
    at loadExtends (/Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/resolve-extends/lib/index.js:39:16)
    at Object.resolveExtends [as default] (/Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/resolve-extends/lib/index.js:25:22)
    at Object.load [as default] (/Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/load/lib/load.js:39:47)
    at async main (/Users/ssbarnea/.cache/pre-commit/repo7yep2n03/node_env-system/lib/node_modules/commitlint-pre-commit-hook/node_modules/@commitlint/cli/lib/cli.js:168:20) {
  code: 'MODULE_NOT_FOUND'
}

FAIL: 1

If you can point me to a repository where this works it would be awesome as I am trying to get rid of husky as having both husky and pre-commit inside the same repo is a bit of a PITA.

Kurt-von-Laven · 2021-10-15T17:11:24Z

The example lists commitlint as a dependency, and those belong in the repo that defines the hook. Not sure whether that will solve your problem, but it seems worth a try if you haven't already.

pkoch · 2021-10-24T15:36:47Z

@ssbarnea Pretty sure this is a stages problem. Can you tell me if this note helps?

If you see things running twice, that's a sign you have misconfigured stages. Be sure to check pre-commit's docs on stages. If this is the first time you're thinking about stages, you're probably only missing a top-level default_stages: [commit]

pkoch · 2021-10-24T15:39:08Z

Sorry, folks, but I'm not finding the right time in my life to invest in here. I'd rather be honest about it and close this for now. :/

escapedcat · 2021-10-25T02:19:57Z

Hey @pkoch , no worries! Take care!

pkoch mentioned this pull request Aug 12, 2021

Default install arguments on .pre-commit-config.yaml, because --hook-types again pre-commit/pre-commit#2007

Closed

pkoch closed this Aug 22, 2021

pkoch reopened this Aug 23, 2021

pkoch changed the title ~~feat: add .pre-commit-hooks.yaml~~ docs: add instructions for pre-commit Aug 23, 2021

docs: add instructions for pre-commit

4c721dc

pkoch mentioned this pull request Aug 23, 2021

chore: add .npmrc to force legacy peer resolution #2729

Merged

escapedcat requested review from AdeAttwood and escapedcat August 23, 2021 02:50

escapedcat approved these changes Aug 23, 2021

View reviewed changes

escapedcat self-requested a review August 24, 2021 05:02

AdeAttwood mentioned this pull request Aug 27, 2021

Add Gitlab CI docs #2736

Closed

pkoch closed this Oct 24, 2021

pkoch deleted the add-pre-commit-hook branch October 24, 2021 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: add instructions for pre-commit #2715

docs: add instructions for pre-commit #2715

pkoch commented Aug 11, 2021 •

edited

Loading

escapedcat commented Aug 12, 2021 •

edited

Loading

AdeAttwood commented Aug 12, 2021

pkoch commented Aug 20, 2021

pkoch commented Aug 21, 2021 •

edited

Loading

pkoch commented Aug 21, 2021

pkoch commented Aug 21, 2021 •

edited

Loading

Mouvedia commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 23, 2021

escapedcat commented Aug 23, 2021

pkoch commented Aug 23, 2021

pkoch commented Aug 28, 2021

Kurt-von-Laven commented Sep 22, 2021

pkoch commented Sep 22, 2021

Kurt-von-Laven commented Sep 22, 2021

ssbarnea commented Oct 15, 2021

Kurt-von-Laven commented Oct 15, 2021

pkoch commented Oct 24, 2021

pkoch commented Oct 24, 2021

escapedcat commented Oct 25, 2021

docs: add instructions for pre-commit #2715

docs: add instructions for pre-commit #2715

Conversation

pkoch commented Aug 11, 2021 • edited Loading

Description

Motivation and Context

Usage examples

How Has This Been Tested?

Types of changes

Checklist:

escapedcat commented Aug 12, 2021 • edited Loading

AdeAttwood commented Aug 12, 2021

Example of the new header from

pkoch commented Aug 20, 2021

pkoch commented Aug 21, 2021 • edited Loading

pkoch commented Aug 21, 2021

pkoch commented Aug 21, 2021 • edited Loading

Mouvedia commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 22, 2021

pkoch commented Aug 23, 2021

escapedcat commented Aug 23, 2021

pkoch commented Aug 23, 2021

pkoch commented Aug 28, 2021

Kurt-von-Laven commented Sep 22, 2021

pkoch commented Sep 22, 2021

Kurt-von-Laven commented Sep 22, 2021

ssbarnea commented Oct 15, 2021

Kurt-von-Laven commented Oct 15, 2021

pkoch commented Oct 24, 2021

pkoch commented Oct 24, 2021

escapedcat commented Oct 25, 2021

pkoch commented Aug 11, 2021 •

edited

Loading

escapedcat commented Aug 12, 2021 •

edited

Loading

pkoch commented Aug 21, 2021 •

edited

Loading

pkoch commented Aug 21, 2021 •

edited

Loading