ci(lint): expand scripts and config lint coverage#115
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's GuideExpands linting and formatting coverage to include CI/tooling scripts and root config files, updates ESLint configuration for script/config scope, and adds unit tests to guard lint/format gates and ESLint config behavior while applying Prettier normalization to affected scripts. Sequence diagram for updated lint npm script pipelinesequenceDiagram
actor Developer
participant NPM as npm_cli
participant PKG as package_json_scripts
participant Prettier as prettier_cli
participant ESLint as eslint_cli
participant MDLinks as markdown_links_linter
participant MDStyle as markdown_style_linter
participant Catalog as test_catalog_validator
participant Changelog as changelog_validator
Developer->>NPM: run lint
NPM->>PKG: resolve script lint
PKG-->>NPM: npm run format:check && eslint ... && npm run lint:md && npm run test:catalog && npm run changelog:validate
rect rgb(230,230,255)
NPM->>PKG: run format:check
PKG-->>Prettier: prettier --check **/*.{json,md,html,css}
Prettier-->>NPM: formatting ok or error
NPM->>Prettier: prettier --check scripts/**/*.js *.config.js eslint.config.js .eslintrc.js .babelrc.js jest.config.js postcss.config.js prettier.config.js tailwind.config.js webpack.config.js playwright.config.ts
Prettier-->>NPM: formatting ok or error
end
NPM->>ESLint: eslint src tests scripts eslint.config.js .eslintrc.js .babelrc.js babel.config.js jest.config.js playwright.config.ts postcss.config.js prettier.config.js tailwind.config.js webpack.config.js --cache --max-warnings 0
ESLint-->>NPM: lint success or failure
NPM->>PKG: run lint:md
PKG-->>MDLinks: node scripts/lint-markdown-links.js
MDLinks-->>NPM: links validation result
PKG-->>MDStyle: markdownlint **/*.{md,mdx}
MDStyle-->>NPM: markdown style result
NPM->>Catalog: node scripts/validate-test-catalog.js
Catalog-->>NPM: catalog validation result
NPM->>Changelog: node scripts/validate-changelog.js
Changelog-->>NPM: changelog validation result
NPM-->>Developer: lint pipeline pass or fail
Class diagram for updated ESLint configuration and lint gatesclassDiagram
class ESLintConfig {
+Array overrides
}
class IgnoreGlobsOverride {
+Array files
+Array ignores
}
class JSRecommendedOverride {
+String source js.configs.recommended
}
class ScriptsAndConfigsOverride {
+Array files
+Object languageOptions
+Object globals
+Object rules
}
class SourceAndTestsOverride {
+Array files
+Object languageOptions
+Object parserOptions
+Object globals
+Object settings
+Object rules
}
class PackageJsonScripts {
+String lint
+String format_check
+String lint_md
+String lint_md_links
+String lint_md_style
+String test_catalog
+String changelog_validate
}
class LintGatesTests {
+test_lint_includes_scripts_and_configs()
+test_format_check_covers_scripts_and_configs()
+test_eslint_config_applies_script_scope_overrides()
}
ESLintConfig o-- IgnoreGlobsOverride
ESLintConfig o-- JSRecommendedOverride
ESLintConfig o-- ScriptsAndConfigsOverride
ESLintConfig o-- SourceAndTestsOverride
PackageJsonScripts --> ESLintConfig : uses
LintGatesTests --> PackageJsonScripts : asserts_scripts
LintGatesTests --> ESLintConfig : asserts_overrides
ScriptsAndConfigsOverride : files = ["scripts/**/*.js", "*.config.js", "eslint.config.js", ".eslintrc.js", ".babelrc.js", "tests/.eslintrc.js"]
ScriptsAndConfigsOverride : languageOptions.ecmaVersion = 2022
ScriptsAndConfigsOverride : languageOptions.sourceType = commonjs
ScriptsAndConfigsOverride : rules.no_unused_vars = off
ScriptsAndConfigsOverride : rules.no_case_declarations = off
ScriptsAndConfigsOverride : rules.no_useless_escape = off
IgnoreGlobsOverride : ignores includes scripts/** (removed)
PackageJsonScripts : lint runs eslint on src tests scripts and config files
PackageJsonScripts : format_check runs prettier on scripts and config files
Flow diagram for expanded format:check Prettier coverageflowchart TD
A[start format_check_script] --> B[run_prettier_check_json_md_html_css]
B -->|success| C[run_prettier_check_scripts_js]
B -->|failure| Z[exit_with_error]
C --> D[run_prettier_check_root_config_js]
D --> E[run_prettier_check_eslint_and_rc_configs]
E --> F[run_prettier_check_jest_and_postcss]
F --> G[run_prettier_check_prettier_tailwind_webpack]
G --> H[run_prettier_check_playwright_ts]
H -->|all_pass| Y[return_success]
H -->|any_fail| Z[exit_with_error]
subgraph script_and_config_targets
C
D
E
F
G
H
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Review Summary by QodoExpand lint and format coverage to scripts and config files
WalkthroughsDescription• Expand ESLint coverage to include scripts and root config files • Extend Prettier formatting checks to JS/TS scripts and config files • Add guard tests for lint/format gate wiring and script-scope config • Apply Prettier normalization to affected scripts now covered by gates Diagramflowchart LR
A["ESLint Config"] -->|"Remove scripts/** ignore"| B["Include scripts & configs"]
C["package.json lint script"] -->|"Add scripts and config files"| D["Expanded ESLint scope"]
E["package.json format:check"] -->|"Add scripts and config files"| F["Expanded Prettier scope"]
G["Script files"] -->|"Apply Prettier formatting"| H["Normalized code style"]
I["New test file"] -->|"Guard lint/format gates"| J["lint-gates.test.js"]
File Changes1. eslint.config.js
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughESLint configuration now includes scripts and configuration files by removing top-level ignores and adding file-specific overrides with CommonJS settings. Package.json lint and format commands expanded to check scripts and config files. Multiple script files reformatted for consistency. Test coverage added for lint/format gates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Mehdi-Bl, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's code quality infrastructure by broadening the scope of ESLint and Prettier checks. It ensures that utility scripts and various configuration files adhere to established coding standards and formatting guidelines. The changes also include new tests to safeguard these expanded linting and formatting gates, preventing regressions and maintaining code consistency across the repository. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The updated
lintandformat:checkscripts inpackage.jsonhardcode a long list of config filenames, which will be easy to forget when adding new tooling files; consider using a more general glob (e.g.*.{config,rc}.jsor a dedicatedconfig/**/*.jsdirectory) to keep this DRY and less brittle over time. - The eslint override for scripts/config files currently disables
no-unused-vars,no-case-declarations, andno-useless-escapeglobally for those paths; it may be worth tightening this (e.g. using/* eslint-disable */in specific legacy files or limiting the override to known-problematic files) so real issues in newer scripts aren’t silently ignored.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The updated `lint` and `format:check` scripts in `package.json` hardcode a long list of config filenames, which will be easy to forget when adding new tooling files; consider using a more general glob (e.g. `*.{config,rc}.js` or a dedicated `config/**/*.js` directory) to keep this DRY and less brittle over time.
- The eslint override for scripts/config files currently disables `no-unused-vars`, `no-case-declarations`, and `no-useless-escape` globally for those paths; it may be worth tightening this (e.g. using `/* eslint-disable */` in specific legacy files or limiting the override to known-problematic files) so real issues in newer scripts aren’t silently ignored.
## Individual Comments
### Comment 1
<location> `eslint.config.js:45-48` </location>
<code_context>
js.configs.recommended,
+ {
+ files: [
+ 'scripts/**/*.js',
+ '*.config.js',
+ 'eslint.config.js',
+ '.eslintrc.js',
+ '.babelrc.js',
+ 'tests/.eslintrc.js',
+ ],
+ languageOptions: {
+ ecmaVersion: 2022,
+ sourceType: 'commonjs',
+ globals: {
+ ...globals.node,
+ ...globals.browser,
+ },
+ },
</code_context>
<issue_to_address>
**suggestion:** Consider avoiding browser globals for Node-only scripts to keep linting stricter and more accurate.
This config enables both Node and browser globals for all `scripts/**/*.js`, even though these files are primarily Node CLI/utility scripts. That can hide accidental use of browser-only APIs in Node code. Prefer keeping `scripts/**/*.js` Node-only and scoping browser globals only to the few files that truly run in a browser context (if any).
```suggestion
globals: {
...globals.node,
},
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great step towards improving code quality and consistency by expanding ESLint and Prettier coverage to include scripts and configuration files. The addition of guard tests to prevent regressions in this coverage is also an excellent practice. My review includes a few suggestions to further improve the new ESLint configuration and simplify the updated npm scripts for better maintainability.
Comment on lines
+50
to
+54
| rules: { | ||
| 'no-unused-vars': 'off', | ||
| 'no-case-declarations': 'off', | ||
| 'no-useless-escape': 'off', | ||
| }, |
There was a problem hiding this comment.
Disabling these core ESLint rules is generally discouraged as it can hide potential bugs and reduce code quality.
no-unused-vars: Instead of turning this off completely, consider configuring it to ignore variables with a specific pattern (e.g., prefixed with an underscore) for intentionally unused variables. This helps catch accidentally unused variables. For example:'no-unused-vars': ['error', { 'argsIgnorePattern': '^_', 'varsIgnorePattern': '^_' }].no-case-declarations: This rule prevents bugs from lexical declarations being visible acrosscaseclauses. Instead of disabling it, you can wrap the code within eachcaseclause in a block{ ... }.no-useless-escape: This rule helps maintain cleaner code by removing unnecessary escape characters. It's usually safe to keep it enabled.
If these rules were disabled to facilitate the initial rollout of linting for these files, it would be beneficial to create a follow-up ticket to address the reported issues and re-enable these rules.
| "dev": "node scripts/index.js dev", | ||
| "clear-assets": "rimraf src/renderer/bundle.js src/renderer/bundle.js.map src/renderer/bundle.js.LICENSE.txt src/renderer/output.css", | ||
| "lint": "npm run format:check && eslint src tests --cache --max-warnings 0 && npm run lint:md && npm run test:catalog && npm run changelog:validate", | ||
| "lint": "npm run format:check && eslint src tests scripts eslint.config.js .eslintrc.js .babelrc.js babel.config.js jest.config.js playwright.config.ts postcss.config.js prettier.config.js tailwind.config.js webpack.config.js --cache --max-warnings 0 && npm run lint:md && npm run test:catalog && npm run changelog:validate", |
There was a problem hiding this comment.
The list of config files in the lint script is quite long. You can simplify this by using globs, which would make the script shorter and easier to maintain. For example, "*.config.js" covers many of the explicitly listed files.
Suggested change
| "lint": "npm run format:check && eslint src tests scripts eslint.config.js .eslintrc.js .babelrc.js babel.config.js jest.config.js playwright.config.ts postcss.config.js prettier.config.js tailwind.config.js webpack.config.js --cache --max-warnings 0 && npm run lint:md && npm run test:catalog && npm run changelog:validate", | |
| "lint": "npm run format:check && eslint src tests scripts \"*.config.js\" \".*rc.js\" \"playwright.config.ts\" --cache --max-warnings 0 && npm run lint:md && npm run test:catalog && npm run changelog:validate", |
| "lint:tests": "eslint tests --cache --max-warnings 0", | ||
| "format": "prettier --write \"**/*.{js,jsx,ts,tsx,json,md,html,css}\"", | ||
| "format:check": "prettier --check --end-of-line auto \"**/*.{json,md,html,css}\"", | ||
| "format:check": "prettier --check --end-of-line auto \"**/*.{json,md,html,css}\" && prettier --check --end-of-line auto \"scripts/**/*.js\" \"*.config.js\" \"eslint.config.js\" \".eslintrc.js\" \".babelrc.js\" \"jest.config.js\" \"postcss.config.js\" \"prettier.config.js\" \"tailwind.config.js\" \"webpack.config.js\" \"playwright.config.ts\"", |
There was a problem hiding this comment.
This script can be simplified. The two prettier commands can be combined into one, and the list of config files can be shortened using globs. This improves readability and maintainability.
Suggested change
| "format:check": "prettier --check --end-of-line auto \"**/*.{json,md,html,css}\" && prettier --check --end-of-line auto \"scripts/**/*.js\" \"*.config.js\" \"eslint.config.js\" \".eslintrc.js\" \".babelrc.js\" \"jest.config.js\" \"postcss.config.js\" \"prettier.config.js\" \"tailwind.config.js\" \"webpack.config.js\" \"playwright.config.ts\"", | |
| "format:check": "prettier --check --end-of-line auto \"**/*.{json,md,html,css}\" \"scripts/**/*.js\" \"*.config.js\" \".*rc.js\" \"*.config.ts\"", |
Code Review by Qodo
1. Browser globals too broad
|
|
Contributor
Author
|
Follow-up on review comments:
Reason for defer: this change set was scoped to expanding gate coverage with zero behavior drift. Re-enabling stricter rules and broadening glob patterns can introduce cross-platform/tooling regressions and should be handled as a dedicated hardening pass with targeted cleanup commits. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n- expand ESLint coverage to include plus root config files used by CI/tooling\n- extend formatter checks to include JS/TS for scripts and config files\n- add guard tests for lint/format gate wiring and script-scope config\n- apply Prettier normalization on affected scripts now covered by the gate\n\n## Validation\n-
Checking formatting...
All matched files use Prettier code style!
Checking formatting...
All matched files use Prettier code style!
Markdown docs lint passed: 18 markdown files checked, 11 links/images scanned, no decorative icons found.
Test catalog validation passed (33 references, 31 discovered tests).
Changelog validation passed: CHANGELOG.md\n-
Loaded environment variables: SONAR_SCANNER_HOME, DTRACK_BASE_URL, VAULT_AGENT_TOKEN_SINK_FILE, VAULT_TOKEN, SONAR_SCANNER_VERSION, SONAR_URL, VAULT_AGENT_BOOTSTRAP_TOKEN_FILE, VAULT_ADDR, VAULT_TOKEN_FILE, VAULT_AGENT_TIMEOUT_SECONDS, VAULT_AGENT_ENV_FILE, VAULT_AGENT_RUNTIME_DIR
Starting SonarQube scan...
SonarQube Server: https://
SONAR_TOKEN not set; attempting unauthenticated scan
Generating fresh Jest coverage report for SonarQube...
-----------------------|---------|----------|---------|---------|--------------------------------------------------------------------------------------------------------------------------------------------
Summary by Sourcery
Expand linting and formatting coverage to include scripts and configuration files and add guards to keep CI lint gates aligned.
Enhancements:
Build:
Documentation:
Tests:
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores