feat: migrate generated packages to root ESLint config and ignore current violations#8067
feat: migrate generated packages to root ESLint config and ignore current violations#8067shivanee-p wants to merge 14 commits intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a baselining mechanism for ESLint violations, including a new script baseline-from-output.mjs to automate the addition of eslint-disable comments and updates to the ESLint configuration. While the goal is to manage existing linting errors across the monorepo, the review identifies several critical issues in the script's implementation. Specifically, the script is susceptible to Out-Of-Memory (OOM) errors when processing large files because it reads entire contents into memory. Furthermore, the regex-based parsing of ESLint output is unreliable, as evidenced by malformed comments in the diff that contain code snippets instead of valid rule IDs. It is highly recommended to refactor the script to use streams for file I/O and ESLint's JSON formatter for robust data extraction.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request centralizes ESLint configuration by removing individual package-level configurations and adding a root-level override in .eslintrc.json. It also introduces a baseline-from-output.mjs script designed to automate the addition of eslint-disable comments to files based on ESLint JSON output, which has been applied across numerous generated client files to suppress prettier/prettier and eol-last warnings. Feedback is provided to ensure the baselining script produces deterministic results by sorting rule IDs and to optimize the TypeScript project configuration for better performance and reliability within a monorepo structure.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request integrates Prettier formatting into the TypeScript GAPIC generator and adds a script to baseline ESLint violations across packages. The baselining script was criticized for using blocking synchronous I/O and for having fragile logic that doesn't account for shebangs or existing headers. Additionally, it was suggested to include the file path in the Prettier formatting call to improve configuration inference.
|
|
||
| try { | ||
| console.log(`Running ESLint...`); | ||
| execSync(`node packages/run-eslint-root.mjs ${pkg}`, { stdio: 'inherit' }); |
| execSync(`node packages/run-eslint-root.mjs ${pkg}`, { stdio: 'inherit' }); | ||
|
|
||
| console.log(`Baselining violations...`); | ||
| execSync(`node packages/baseline-from-output.mjs ${pkg}`, { stdio: 'inherit' }); |
This PR migrates the generated libraries in the monorepo to use a centralized ESLint configuration based on Google TypeScript Style (GTS) standards. The previous linter checks were not sufficient to maintain consistent code quality across all packages.
Addresses #7920 🦕
Key Changes
.eslintrc.jsonfiles../node_modules/gts, applying these rules to files inpackages/via overrides.handwritten/directory in the root.eslintignoreto avoid affecting those libraries in this phase.Next Steps