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

chore: remove unused commitlint and husky #1824

Merged

Conversation

trentm
Copy link
Contributor

@trentm trentm commented Nov 23, 2023

Background:

  • Husky and commitlint used to be used in the core and contrib repos.
  • They were removed from the core repo in Remove Husky opentelemetry-js#2078 because the DX was annoying. The changelog is not generated in the core repo, so it doesn't require strict adherence to "conventional commits" syntax to support that tooling.
  • A ".github/workflows/pr-title.yml" was added to the contrib repo in ci: lint PR titles #1068 to help ensure merged commits follow "conventional commits" (partly to help with changelog generation tooling).
  • Later, as a rider on chore: exclude examples from renovate #1460 (comment) the .husky/... config was removed from the contrib repo, again because its DX is just annoying, and unnecessary given the "pr-title" CI workflow.

Currently, the husky and commitlint deps remain in the contrib repo, but they are not used -- except to add some minor annoyances in some dev cases (some examples are below).

This PR removes vestiges of husky and commitlint.

some minor annoyances from the lingering husky and commitlint

Since a recent update of commitlint to a version that requires Node.js 18, when using Node.js 16, npm ci starts out with a wall of EBADENGINE warnings:

% nvm use 16
Now using node v16.20.2 (npm v8.19.4)

% npm ci
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/cli@18.4.1',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/config-conventional@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/config-validator@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/ensure@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/execute-rule@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/format@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/is-ignored@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/lint@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/load@18.4.1',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/message@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/parse@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/read@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/resolve-extends@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/rules@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/to-lines@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/top-level@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@commitlint/types@18.4.0',
npm WARN EBADENGINE   required: { node: '>=v18' },
npm WARN EBADENGINE   current: { node: 'v16.20.2', npm: '8.19.4' }
npm WARN EBADENGINE }
npm WARN deprecated @types/generic-pool@3.8.1: This is a stub types definition. generic-pool provides its own type definitions, so you do not need this installed.
...

In a clean clone (e.g. after git clean -ffdx), running npm install --package-lock-only fails because of the husky usage in the "prepare" script:

% npm install --package-lock-only

> opentelemetry-contrib@0.1.0 prepare
> husky install

sh: husky: command not found
npm ERR! code 127
npm ERR! path /Users/trentm/tm/opentelemetry-js-contrib4
npm ERR! command failed
npm ERR! command sh -c husky install

npm ERR! A complete log of this run can be found in: /Users/trentm/.npm/_logs/2023-11-23T22_23_31_705Z-debug-0.log

@trentm trentm self-assigned this Nov 23, 2023
@trentm trentm requested a review from a team November 23, 2023 22:36
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

Merging #1824 (aab491b) into main (660e37b) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1824   +/-   ##
=======================================
  Coverage   91.50%   91.50%           
=======================================
  Files         144      144           
  Lines        7369     7369           
  Branches     1467     1467           
=======================================
  Hits         6743     6743           
  Misses        626      626           

@trentm trentm merged commit b39c96c into open-telemetry:main Nov 28, 2023
13 checks passed
@trentm trentm deleted the tm-remove-husky-commitlint-vestiges branch November 28, 2023 20:06
@trentm
Copy link
Contributor Author

trentm commented Nov 28, 2023

I just saw the movement on #1829. I hope my merging this hasn't created toil for getting the release out.

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

Successfully merging this pull request may close these issues.

3 participants