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

Split lint commands #907

Closed
wants to merge 37 commits into from
Closed

Split lint commands #907

wants to merge 37 commits into from

Conversation

goastler
Copy link
Member

@goastler goastler commented Dec 11, 2023

fixes prosopo/captcha-private#463

restructures the linting (eslint+prettier) into scripts specific to each package. This allows for overriding how the linting is done per package, what files are included, etc. Also simplifies the top-level linting scripts

blocked by #904

fixes prosopo/captcha-private#515
fixes prosopo/captcha-private#516

blocked by #902

Copy link

Pull Request Review

Hey there! 👋 I've summarized the previous results for you to write a Pull Request review markdown doc. Let's get started!

Changes

Here are the major changes made in this pull request:

  1. Removed 'eslint:recommended' from the 'extends' array in .eslintrc.cjs.
  2. Added 'eslint:recommended' and 'plugin:@typescript-eslint/recommended' to the 'extends' array in .eslintrc.cjs.
  3. Added 'prettier' to the 'extends' array in .eslintrc.cjs.
  4. Removed 'ecmaVersion' from the 'parserOptions' object in .eslintrc.cjs.
  5. Added 'root: true' to .eslintrc.cjs.

Suggestions

Here are some suggestions to improve the code:

  1. In package.json files, replace the 'lint' script with 'eslint' and 'prettier' scripts.
  2. In package.json files, replace the 'lint:fix' script with 'eslint:fix' and 'prettier:fix' scripts.

Bugs

Here are the potential bugs found:

  1. Potential bug in .eslintrc.cjs: 'plugin:prettier/recommended' is commented out, but 'prettier' is added to the 'extends' array.
  2. Potential bug in package.json files: 'lint' script is missing 'prettier' command.

Improvements

Here's an improvement suggestion for better readability:

In the codebase, there are several places where the 'eslint' and 'prettier' scripts are duplicated in the 'scripts' section of package.json files. It would be better to extract these scripts into a separate file and reference it in the 'scripts' section. This would improve readability and maintainability of the codebase.

Example code snippet to extract the 'eslint' and 'prettier' scripts:

{
  "scripts": {
    "eslint": "eslint .",
    "prettier": "prettier ."
  }
}

Rating

The code has been rated on a scale of 0 to 10 based on the following criteria:

  • Readability: 8
  • Performance: 7
  • Security: 9

That's it! Feel free to make any necessary adjustments and add more details if needed. Good luck with your Pull Request! 🚀

P.S. Did you know that we offer a premium plan that can analyze big pull requests? It provides even more in-depth analysis and suggestions. Consider upgrading for an enhanced experience! 😉

@goastler goastler mentioned this pull request Dec 11, 2023
@github-actions github-actions bot removed the blocked label Dec 11, 2023
@goastler goastler enabled auto-merge (squash) December 11, 2023 16:13
Copy link
Contributor

⏳ Blocked by: * #904

@goastler goastler mentioned this pull request Dec 11, 2023
@goastler
Copy link
Member Author

superseded by #909

@goastler goastler closed this Dec 12, 2023
auto-merge was automatically disabled December 12, 2023 09:48

Pull request was closed

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.

1 participant