Skip to content

Create centralized linting system that can be applied to all MatrixAI repos, regardless of project-specific linting rules #7

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

Open
CDeltakai opened this issue Mar 21, 2025 · 18 comments · May be fixed by #16
Assignees
Labels
development Standard development

Comments

@CDeltakai
Copy link
Contributor

Specification

If we can create a single config bundle that includes all of the rules from MatrixAI codebases, we can greatly streamline the process of setting up linting rules in new repos.

Additional context

Tasks

  1. Research into how to implement a bundled config file that can be used to apply rules from other eslint-plugins
  2. Create a bundled config file that combines all of the commonly used rules in our repos in a single package
@CDeltakai CDeltakai added the development Standard development label Mar 21, 2025
Copy link

linear bot commented Mar 21, 2025

ENG-557

@CMCDragonkai
Copy link
Member

Old dependencies can be replaced with just 1:

Image

Image

This repo has to be an ESM repo too.

https://github.com/MatrixAI/Zeta-House-Docs/blob/staging/scripts/lint.mjs - make sure it works with the lint.mjs code.

@CMCDragonkai
Copy link
Member

ETA on this? This has been worked on for a couple weeks now. It should be merged now.

@CMCDragonkai
Copy link
Member

@CDeltakai

@CMCDragonkai
Copy link
Member

I've reviewed very quickly the result of this work, and I don't think this is what we want. I'm going to give a better spec so that this actually some real ROI, which right now it does not.

  1. Every linting rules in every project includes roughly:
    • eslint
    • shellcheck for shell scripts
    • prettier for markdown
    • clang-format for C/C++
    • nix-format for nix
    • rustfmt for rust
  2. This repository shouldn't be exporting an eslint plugin. (It can, but that's not the point of this repo).
  3. This repository SHOULD be exporting a lint command. And this single lint command should automatically run linting rules for all relevant projects/directories, and downstream repos can "extend it" to filter out certain directories or do extra things.

To achieve this.

  1. Rename this repo to be js-lint. That way it represents the totality of linting for the entire organisation.
  2. The package.json should be called @matrixai/lint. See point 1.
  3. This needs a bin program specified in package.json that runs the lint command. This command should be located under src/bin/lint.ts. It can look like this:
    "bin": {
      "lint": "dist/bin/lint.js"
    },
    
  4. The src/bin/lint.ts needs to follow roughly the same structure of "linting" as this script located in zeta.house: https://github.com/MatrixAI/zeta.house/blob/staging/scripts/lint.mjs
  5. This repo's dependencies needs to contain all the relevant eslint dependencies that we use, see zeta.house's package.json for a common set. (Yes I know that PKE has different rules, ignore it for now.)
  6. We're going to incrementally apply this to the js-logger.
  7. There are some "native" dependencies that work as part of the nix develop shell, which brings in shellcheck and clang-format, nixfmt, rustfmt. These dependencies cannot be part of the package.json, because they are native dependencies. The only dependencies that's possible under package.json is eslint and eslint related libraries and prettier.
  8. Since the lint script depends on non-JS packages, and uses things like shellcheck... etc. Then what you need to do is one of the 2 things:
    • Option 1: bundle native dependencies into our npm package
    • Option 2: just "test" for the existence of these commands, and run them optionally IF THEY exist in the environment
    • DO OPTION 2. DO NOT DO OPTION 1. Because OPTION 1 is hacky and npm wasn't designed to distribute native commands. And just is bad vooodoooo..

@CMCDragonkai
Copy link
Member

I've just done:

  1. Rename this repo to be js-lint. That way it represents the totality of linting for the entire organisation.

The rest is handed to you @CDeltakai.

@CMCDragonkai
Copy link
Member

For step 8. As an example https://github.com/MatrixAI/zeta.house/blob/a42d071dcf6813ce5a225b4a699fed67881e6d75/scripts/lint.mjs#L46-L69

You just need to "test" if the program already exists in the shell before doing the rest of the code.

if (commandExists('find') && commandExists('shellcheck')) {
  // ...
}

Ask chatgpt to generate the function for you:

  • It must work on linux, macos, windows
  • It must be very fast
  • It must return true/false
  • It must take a string representing the program I want to check
  • It must check that the program exists in the current environment

@CMCDragonkai
Copy link
Member

Step 9. is to do a release on this repo. Create a 0.0.1 version of this @matrixai/lint.

Step 10. is to create a feature branch in js-logger, and get it working there.

Remember the CI does step 9. You just need to create a "version tag".

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Mar 28, 2025

At step 10. we will remove all eslint and linting programs from that repo. And instead the lint and lintfix scripts will just call lint command coming out of this repo. It is possible to do this because of our nix develop shell.

Or we can even call npx @matrixai/lint or npx @matrixai/lint --fix.

@CDeltakai CDeltakai changed the title Create a bundled config for all of the commonly used lint rules in MatrixAI codebases Create centralized linting system that can be applied to all MatrixAI repo's, regardless of project-specific linting rules Mar 28, 2025
@CDeltakai CDeltakai changed the title Create centralized linting system that can be applied to all MatrixAI repo's, regardless of project-specific linting rules Create centralized linting system that can be applied to all MatrixAI repos, regardless of project-specific linting rules Mar 28, 2025
@CMCDragonkai
Copy link
Member

Create new PRs for all the rest of the repos to start integrating this. Start with js-logger.

@CDeltakai
Copy link
Contributor Author

Can use snapshot testing in jest to test the linter. Otherwise there's no tests.

@CDeltakai
Copy link
Contributor Author

@brynblack Can't finish the CI at the moment since the CI for this repo apparently doesn't have the authority to push out a release.

https://github.com/MatrixAI/js-lint/actions/runs/14463613455

@CMCDragonkai
Copy link
Member

It's all been released now at https://www.npmjs.com/package/@matrixai/lint.

I manually setup the org secrets.

Now create every PR and reference this. Don't fix it, only relates.

I think you also need to make the "paths" configurable at the CLI level.

@CMCDragonkai
Copy link
Member

    "lint": "eslint '{src,tests,scripts,benches}/**/*.{js,mjs,ts,mts,jsx,tsx}'",
    "lintfix": "eslint '{src,tests,scripts,benches}/**/*.{js,mjs,ts,mts,jsx,tsx}' --fix",
    "lint-shell": "find ./src ./tests ./scripts -type f -regextype posix-extended -regex '.*\\.(sh)' -exec shellcheck {} +",
    "lint": "matrixai-lint --eslint='{src,tests,scripts,benches}/**/*.{js,mjs,ts,mts,jsx,tsx}' --shell='{src,tests,scripts}/*.{sh}'",
    "lintfix": "matrixai-lint --fix --eslint='{src,tests,scripts,benches}/**/*.{js,mjs,ts,mts,jsx,tsx}' --shell='{src,tests,scripts}/*.{sh}'",

In the future, additional options can be provided to address other kinds of linting.

The goal is to remove configuration files from all the repos.

@CMCDragonkai
Copy link
Member

The pattern is a "globbing" pattern. You need to "interpret" the globbing pattern in JS. Use a glob library to do this, or ask gemini.

@CMCDragonkai
Copy link
Member

Also if your command is going to be matrixai-lint, your src/bin/lint.ts should also just be src/bin/matrixai-lint too...

Or alternatively you export it as just a lint program. In which case the scripts above should just be lint.

I think it could just be lint. There's no well known program that it conflicts with.

@CMCDragonkai
Copy link
Member

Because the repo is authorized now, you can now proceed to to even merge new changes to staging and push new tags. Remember you are still doing prereleases.

@tegefaulkes
Copy link
Contributor

For globbing I've used https://www.npmjs.com/package/minimatch minimatch before.

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