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.

Sign up for GitHub

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

Jump to bottom

ci: Fix eslint integration #373

Merged
merged 3 commits into from
Apr 7, 2024
Merged

ci: Fix eslint integration #373

merged 3 commits into from
Apr 7, 2024

Commits on Apr 7, 2024

  1. ci: Fix eslint integration 

    In bazel-contrib#363, I accidentally disabled all eslint checks: I used `eslint`
and thought this would actually run the linter on the complete source
tree. Starting from ESLint 9.0, this will actually work (See
https://eslint.org/blog/2024/04/eslint-v9.0.0-released/#running-eslint-with-no-file-arguments)
But for ESLint 8.x, we still need to use `eslint .`

This commit fixes that oversight in `package.json` and rectifies all the
fall-out from this mistake.

First, I finished the migration to the new eslint.config.js format. To
avoid further bugs, I first enabled TypeScript's type checking via
`@ts-check`. I then followed the migration guide from
https://eslint.org/docs/latest/use/configure/migration-guide
to fix the config in general.

Doing so, I changed a the linter config in the following way:
* I disabled the `max-classes-per-file` linter rule. My change bazel-contrib#365
  violated this rule. But I don't think that this rule provides much
  value in general. Having one class per file is rather a Java
  convention than a JavaScript convention...
* I replaced `prefer-arrow/prefer-arrow-functions` by the built-in
  `prefer-arrow-callback`. Afaict, those checks do the same. This
  allowed me to remove the dependency on the package
  `eslint-plugin-prefer-arrow`.
* The linting actually alos applied to the `eslint.config.js` file
  itself. But  not all packages have types, zet. I hence disabled a
  couple of rules for this file.

With that out-of-the way, I had to fix the actual lint warnings:
* In `eslint-plugin-prefer-arrow bazel_workspace_info.ts and
  buildifier.ts: "Unexpected lexical declaration in case block"
* extension.ts had an (intentionally) dangling promise
* debug-adapter/connection.ts had an intentional `while(true)`
  loop.
    @vogelsgesang
    vogelsgesang committed Apr 7, 2024
    Configuration menu
    Copy the full SHA
    6174d36 View commit details
    Browse the repository at this point in the history

  2. Fix compilation

    @vogelsgesang
    vogelsgesang committed Apr 7, 2024
    Configuration menu
    Copy the full SHA
    82497ab View commit details
    Browse the repository at this point in the history

  3. Actually remove prefer-arrow plugin dependency

    @vogelsgesang
    vogelsgesang committed Apr 7, 2024
    Configuration menu
    Copy the full SHA
    f2bd7d5 View commit details
    Browse the repository at this point in the history