-
Notifications
You must be signed in to change notification settings - Fork 562
improvement(build-tools): address ESLint rule violations in build-tools #26152
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
base: main
Are you sure you want to change the base?
Conversation
Fix low-count ESLint violations across the build-tools workspace: - Replace .replace(/x/g, ...) with .replaceAll() (unicorn/prefer-string-replace-all) - Change import * as path to import path (unicorn/import-style) - Remove useless undefined arguments (unicorn/no-useless-undefined) - Fix JSDoc @param names to match actual parameters (jsdoc/check-param-names) - Add inline disables for idiomatic usage patterns: - async.mapLimit, semver.parse, sortJson.overwrite (import-x/no-named-as-default-member) - prompts default export (import-x/no-named-as-default) - Remove unused eslint-disable directives for import-x/no-default-export - Change @module to @packageDocumentation for TSDoc compatibility (tsdoc/syntax)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses ESLint rule violations across the build-tools workspace by fixing code to comply with newly enabled ESLint rules, without modifying the ESLint configuration itself.
- Modernizes string replacement operations using
.replaceAll()instead of regex-based.replace(/x/g, ...) - Updates import statements to use default imports for Node.js built-in modules like
path - Removes unnecessary
undefinedarguments and fixes JSDoc parameter names - Adds appropriate inline ESLint disable comments for idiomatic library usage patterns
- Updates TSDoc annotations for better compatibility
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
build-tools/packages/version-tools/src/internalVersionScheme.ts |
Removes useless undefined arguments from validateVersionScheme calls |
build-tools/packages/version-tools/src/commands/version/latest.ts |
Removes unused import-x/no-default-export disable directive |
build-tools/packages/version-tools/src/commands/version.ts |
Removes unused import-x/no-default-export disable directive |
build-tools/packages/build-infrastructure/src/test/packageJsonUtils.test.ts |
Changes path import from namespace to default import |
build-tools/packages/build-infrastructure/src/test/git.test.ts |
Changes path import from namespace to default import |
build-tools/packages/build-infrastructure/src/index.ts |
Updates @module to @packageDocumentation for TSDoc compatibility |
build-tools/packages/build-infrastructure/src/buildProject.ts |
Replaces .replace(/\\/g, "/") with .replaceAll("\\", "/") |
build-tools/packages/build-infrastructure/api-report/build-infrastructure.api.md |
Removes auto-generated comment about missing package documentation |
build-tools/packages/build-cli/src/commands/release/report.ts |
Fixes JSDoc parameter names and adds inline disable for sortJson.overwrite |
build-tools/packages/build-cli/src/commands/release/report-unreleased.ts |
Adds missing JSDoc parameter documentation |
build-tools/packages/build-cli/src/commands/release/fromTag.ts |
Adds inline disable for semver.parse |
build-tools/packages/build-cli/src/commands/generate/typetests.ts |
Replaces .replace(/x/g, ...) with .replaceAll() |
build-tools/packages/build-cli/src/commands/generate/changeset.ts |
Adds inline disable for prompts default import |
build-tools/packages/build-cli/src/commands/generate/assertTags.ts |
Replaces .replace(/x/g, ...) with .replaceAll() |
build-tools/packages/build-cli/src/commands/check/policy.ts |
Changes path import and replaces .replace() with .replaceAll() |
build-tools/packages/build-cli/src/commands/bump/deps.ts |
Adds inline disable for prompts default import |
build-tools/packages/build-cli/src/codeCoverage/getCoverageMetrics.ts |
Fixes JSDoc parameter name |
build-tools/packages/build-cli/src/BasePackageCommand.ts |
Adds inline disable for async.mapLimit |
Co-authored-by: Copilot <[email protected]>
|
|
||
| import { Flags } from "@oclif/core"; | ||
| import chalk from "picocolors"; | ||
| // eslint-disable-next-line import-x/no-named-as-default -- prompts default export is the intended API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lines before and after this one also import the default exports of the corresponding packages; if this is needed on this line, it must be needed on the other two, no?
| import { PackageName } from "@rushstack/node-core-library"; | ||
| import { humanId } from "human-id"; | ||
| import chalk from "picocolors"; | ||
| // eslint-disable-next-line import-x/no-named-as-default -- prompts default export is the intended API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as the previous comment, the line above this follows the same pattern so should need a disable too?
These changes are in preparation for upgrading to eslint 9 in build-tools.
Summary
Changes
.replace(/x/g, ...)with.replaceAll()(unicorn/prefer-string-replace-all)import * as pathtoimport path(unicorn/import-style)undefinedarguments (unicorn/no-useless-undefined)@paramnames to match actual parameters (jsdoc/check-param-names)async.mapLimit,semver.parse,sortJson.overwrite(import-x/no-named-as-default-member)promptsdefault export (import-x/no-named-as-default)@moduleto@packageDocumentationfor TSDoc compatibility (tsdoc/syntax)