QA (public) » openQA Project (public)

I wouldn't mind taking care of this. Unless someone else would like to use the opportunity to learn more about JavaScript dev tools. My recommendation would be:

1. Put a package.json and eslint.json into the repo that contain all the deps and settings (semi-standard + extended max line length)
2. Create a tools/eslint script that will check if node is installed (give a useful message otherwise), update deps with npm in the local node_modules, and then run eslint --fix over our JavaScript files (make sure it complains about violations it can't fix)
3. Add a GitHub Action that runs eslint over our JavaScript files and annotates pull requests with rule violations
4. Make sure all the temporary npm files are added to .gitignore
5. Update relevant documentation

Doing it this way also means that clever editors such as VSCode can use the settings to lint your code as you write it.

Just one thing comes to mind that you didn't mention. Make sure that we have a makefile target that does the linting and which is used in our CI - I don't care that much about installing npm and such here, but it should be trivial to reproduce whatever we do in CI without looking up command lines to execute. This has been a source of problems before. From a glance the mentioned GHA runs 3 different commands.

cdywan wrote:

Just one thing comes to mind that you didn't mention. Make sure that we have a makefile target that does the linting and which is used in our CI - I don't care that much about installing npm and such here, but it should be trivial to reproduce whatever we do in CI without looking up command lines to execute. This has been a source of problems before. From a glance the mentioned GHA runs 3 different commands.

Actually i think a separate GitHub Action would be better, since those can annotate the pull request diff with eslint rule violations.

I've prepared a very large draft PR that replaces jsbeautifier with eslint and semistandard rules + GitHub Action. https://github.com/os-autoinst/openQA/pull/3900

The PR also demonstrates how eslint is about more than just formatting, it's a proper linter that can find many other issues for us.

I think one thing the eslint results have shown is that our JavaScript code is in a very poor state. Merely running a prettifier over it is not gonna solve that, we also need to make a decision how we want to organise it (ESM etc.).

Here's another idea, ESLint with Prettier formatting rules. That has the advantage that we can start only with formatting rules and later add more. https://github.com/os-autoinst/openQA/pull/4042

This commit shows one ESLint rule being enabled as an error. More can follow the same way, or be enabled with a category such as eslint:recommended.

kraih wrote:

[…] That should resolve this ticket.

ok, but why did you not "resolve" it then? Do you think we need more rules enforced?

kraih wrote:

Fair point, if i leave this unresolved it could stay open forever since there will always be new ESLint rules for us to enable in the future.

makes sense, thanks. We can update/add rules over time.

Made a followup PR with the most important rules from eslint:recommended. https://github.com/os-autoinst/openQA/pull/4088