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

pre-commit: add eslint-plugin-jsdoc #4580

Merged
merged 1 commit into from
Dec 22, 2021

Conversation

Swiftb0y
Copy link
Member

This allows us to enforce that the libraries we ship
come with good JSDoc comments which should improve DX
when mapping with capable code editors.

This allows us to enforce that the libraries we ship
come with good JSDoc comments which should improve DX
when mapping with capable code editors.
@Holzhaus
Copy link
Member

Won't this prevent any changes to the first midi-components library, the common controller scripts or the hid package parser? Or do you plan to add jsdoc comments to all of these?

@Swiftb0y
Copy link
Member Author

From my tests so far, it only actually enforces jsdoc comment style. If there is no jsdoc comment, the test just passes. I think this is because it can't detect what symbols we're actually exporting currently. Once we convert some of the modules to proper ES modules, the jsdoc hook should work. In the meantime, it just scans source files for jsdoc comments and complains about those but does not force that everything needs to have a comment.

@Holzhaus Holzhaus added this to the 2.4.0 milestone Dec 22, 2021
@Holzhaus Holzhaus merged commit 1feea5f into mixxxdj:main Dec 22, 2021
@JoergAtGithub
Copy link
Member

My PR #4583 fails with:
grafik
I guess this is related to this change.

@Swiftb0y
Copy link
Member Author

mhmm pretty sure it isn't. I guess your pre-commit environment got initialized incorrectly. Maybe try pre-commit clean or something.

@Swiftb0y
Copy link
Member Author

Swiftb0y commented Dec 31, 2021

@JoergAtGithub I reinstalled pre-commit completely and ran it on an example file. eslint works as expected. I'm pretty sure the pre-commit failures are unrelated to this PR.

[INFO] Initializing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Initializing environment for https://github.com/codespell-project/codespell.
[INFO] Initializing environment for https://github.com/pre-commit/mirrors-eslint.
[INFO] Initializing environment for https://github.com/pre-commit/mirrors-eslint:eslint-plugin-jsdoc@^v37.4.0.
[INFO] Initializing environment for local.
[INFO] Initializing environment for https://github.com/psf/black.
[INFO] Initializing environment for https://gitlab.com/pycqa/flake8.
[INFO] Initializing environment for https://github.com/shellcheck-py/shellcheck-py.
[INFO] Initializing environment for https://github.com/DavidAnson/markdownlint-cli2.
[INFO] Initializing environment for local:tinycss==0.4.
[INFO] Initializing environment for local:Markdown==3.3.4,beautifulsoup4==4.9.3,lxml==4.6.3.
[INFO] Installing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/codespell-project/codespell.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/pre-commit/mirrors-eslint.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/psf/black.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://gitlab.com/pycqa/flake8.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/shellcheck-py/shellcheck-py.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/DavidAnson/markdownlint-cli2.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for local.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
fix UTF-8 byte order marker..............................................Passed
Check for case conflicts.................................................Passed
Check JSON...........................................(no files to check)Skipped
Check for merge conflicts................................................Passed
Check Xml............................................(no files to check)Skipped
Check Yaml...........................................(no files to check)Skipped
Fix End of Files.........................................................Passed
Mixed line ending........................................................Passed
Trim Trailing Whitespace.............................(no files to check)Skipped
Don't commit to branch...................................................Passed
codespell................................................................Passed
eslint...................................................................Failed
- hook id: eslint
- exit code: 1


/home/swiftb0y/Sourcerepositories/mixxx/res/controllers/midi-components-0.0.js
  294:13  error    'print' is not defined                      no-undef
  327:17  error    'print' is not defined                      no-undef
  362:13  error    'print' is not defined                      no-undef
  499:0   warning  Missing JSDoc @param "options" description  jsdoc/require-param-description
  499:0   warning  Missing JSDoc @param "options" type         jsdoc/require-param-type
  514:17  error    'print' is not defined                      no-undef
  520:13  warning  Unexpected var, use let or const instead    no-var
  540:17  error    'print' is not defined                      no-undef
  546:13  warning  Unexpected var, use let or const instead    no-var
  678:13  error    'print' is not defined                      no-undef
  827:13  error    'print' is not defined                      no-undef

✖ 11 problems (7 errors, 4 warnings)


clang-format.........................................(no files to check)Skipped
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
shellcheck...........................................(no files to check)Skipped
markdownlint-cli2....................................(no files to check)Skipped
qsscheck.............................................(no files to check)Skipped
changelog............................................(no files to check)Skipped
qmlformat............................................(no files to check)Skipped
qmllint..............................................(no files to check)Skipped
metainfo.............................................(no files to check)Skipped

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

Successfully merging this pull request may close these issues.

3 participants