Repository navigation
feat: add basic monorepo linter to enforce GTS standards - #8446
Conversation
![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 introduces a new linter script (bin/linter.mjs) that uses Prettier to check formatting on changed files, and updates package.json to run this script during the lint step. The review feedback highlights critical security and robustness improvements: specifically, replacing execSync with execFileSync to prevent command injection vulnerabilities and handle filenames with spaces or special characters safely, and dynamically resolving the base branch using process.env.GITHUB_BASE_REF to support CI environments.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new linter script, bin/linter.mjs, which runs Prettier to check formatting on changed files against a base branch, and updates the lint script in package.json to execute it. Feedback on the implementation highlights two issues: potential failures in CI environments if the local base branch is missing (suggesting a fallback to the remote tracking branch), and cross-platform compatibility issues on Windows when executing npx via execFileSync (suggesting dynamically resolving the command to npx.cmd).
c7ebcda to
efb3794
Compare
154c45f to
d9dac40
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new linter script (bin/linter.mjs) that checks changed TypeScript files using Prettier and ESLint, and updates package.json to run this script. The review feedback highlights several robustness issues when running the script from subdirectories or within CI environments. It recommends dynamically resolving the repository root directory, executing Git and the linter binaries relative to this root, and using remote tracking branches for the base branch in CI.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a custom linter script (bin/linter.mjs) that programmatically runs Prettier and ESLint on changed TypeScript files, and updates package.json to use this script and include the necessary devDependencies. The review feedback suggests several improvements to make the linter more robust for CI environments: dynamically resolving the git diff reference to prevent failures when the base branch is not fetched, rethrowing programmatic ESLint errors to avoid silent failures, and running Prettier and ESLint sequentially rather than concurrently to ensure clean console output and complete execution of both tools.
…gs, and add package-level type checking
…dd type safety checks
…figure blocking rules
02e687d to
906c498
Compare
2 participants
This PR refactors the monorepo linter
bin/linter.mjsto run checks on files modified in a PR, with formatting, static linting, and TypeScript compilation checks. This is done via the local CLI binaries directly via execFileSync, avoiding complex programmatic configurations.getGitTarget()to check for the base branch in order of priority:origin/<base>-><base>->HEAD~1. This prevents linter failures in shallow-cloned or detached-HEAD CI environments (like GitHub Actions) and avoids comparing against outdated local branches.plugin:prettier/recommendedin the root .eslintrc.json.eslint-plugin-importandeslint-plugin-promiseto enforce best practices.import/no-extraneous-dependencies,promise/catch-or-return, andpromise/always-returncheckTypeSafety()which runstsc --noEmiton parent packages usingnode_modules/typescript/bin/tscNo TypeScript files changed. Skipping checks.to provide immediate feedback when no .ts files are touched.