feat: adding standard scripts for lint and format check#7326
feat: adding standard scripts for lint and format check#7326sriramveeraghanta merged 6 commits intopreviewfrom
Conversation
|
Caution Review failedThe pull request is closed. """ WalkthroughThe changes restructure and standardize code quality scripts and Turbo task configurations across multiple apps and the root project. Linting, formatting, and type checking scripts are split into explicit "check" and "fix" commands, with cleanup scripts expanded. The Turbo pipeline is refactored for granular check/fix tasks with disabled caching. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant NPM as npm/yarn
participant Turbo as turbo
participant Lint as ESLint
participant Type as TypeScript
participant Format as Prettier
Dev->>NPM: run check
NPM->>Turbo: turbo run check
Turbo->>Lint: check:lint (ESLint, no warnings)
Turbo->>Type: check:types (type check, no emit)
Turbo->>Format: check:format (Prettier check)
Lint-->>Turbo: Lint results
Type-->>Turbo: Type results
Format-->>Turbo: Format results
Turbo-->>NPM: Aggregate check results
NPM-->>Dev: Output
Dev->>NPM: run fix
NPM->>Turbo: turbo run fix
Turbo->>Lint: fix:lint (ESLint --fix)
Turbo->>Format: fix:format (Prettier --write)
Lint-->>Turbo: Lint fix results
Format-->>Turbo: Format fix results
Turbo-->>NPM: Aggregate fix results
NPM-->>Dev: Output
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/live/package.json (1)
13-18: Userimrafto make thecleanscript cross-platform
rm -rffails on Windows shells and forces contributors to install a Unix layer. Usingrimraf(already very common in JS repos) keeps the command portable and simpler.- "clean": "rm -rf .turbo && rm -rf .next && rm -rf node_modules && rm -rf dist" + "clean": "rimraf .turbo .next node_modules dist"You’ll need to add
rimraftodevDependenciesonce at the workspace root (it will be hoisted).turbo.json (1)
37-60: Re-enable Turbo caching for check tasks to speed up CIDisabling the cache for purely-deterministic tasks (
eslint,prettier --check,tsc --noEmit) forces them to run every time and will noticeably slow large CI matrices. Unless you’ve hit a specific caching bug, consider restoring the default:- "check:types": { - "dependsOn": ["^build"], - "cache": false + "check:types": { + "dependsOn": ["^build"] }, - "check:lint": { - "cache": false + "check:lint": {}, - "check:format": { - "cache": false + "check:format": {},Same for the aggregate
checktask.package.json (1)
17-19: Minor: document whatfixvscheckdo at the rootThe script names are clear to us but not to casual contributors. A short comment in the README (or
"description"fields inscriptsonce Yarn 2+ is adopted) will avoid confusion.No code change required.
apps/admin/package.json (1)
13-18: Same portability concern forcleanhereMirror the earlier suggestion: replace chained
rm -rfcalls with a singlerimrafinvocation.- "clean": "rm -rf .turbo && rm -rf .next && rm -rf node_modules && rm -rf dist", + "clean": "rimraf .turbo .next node_modules dist",Keeps the monorepo consistent across OSes.
apps/web/package.json (1)
11-16: Portability & duplication – adopt a workspace-levelcleanIdentical to the admin & live packages:
- Swap out
rm -rfforrimraffor Windows compatibility.- Consider hoisting the
cleanscript to the root and invoking it with
yarn workspaces foreach -t run cleanto avoid four nearly-identical definitions.- "clean": "rm -rf .turbo && rm -rf .next && rm -rf node_modules && rm -rf dist", + "clean": "rimraf .turbo .next node_modules dist",apps/space/package.json (1)
12-16: Exclude build artefacts from lint/format globs to speed CIRunning ESLint & Prettier over the entire repo (
./**/*) hits.next,dist, and possibly generated code.
Even if these are ignored via.eslintignore/.prettierignore, the glob walk still incurs IO overhead.Consider narrowing the scope:
- "check:lint": "eslint . --max-warnings 0", + "check:lint": "eslint \"src/**/*.{ts,tsx}\" --max-warnings 0", - "check:format": "prettier --check \"**/*.{ts,tsx,md,json,css,scss}\"", + "check:format": "prettier --check \"src/**/*.{ts,tsx,md,css,scss}\"", - "fix:lint": "eslint . --fix", + "fix:lint": "eslint \"src/**/*.{ts,tsx}\" --fix", - "fix:format": "prettier --write \"src/**/*.{ts,tsx,md,css,scss}\"" + "fix:format": "prettier --write \"src/**/*.{ts,tsx,md,css,scss}\""This shaves minutes off CI for large workspaces and avoids accidental formatting changes in vendored/generated files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/admin/package.json(1 hunks)apps/live/package.json(1 hunks)apps/space/package.json(1 hunks)apps/web/package.json(1 hunks)package.json(1 hunks)turbo.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
packages/propel/package.json (1)
7-13: Same clean-script portability issue as noted for@plane/typesSee the previous comment about replacing
rm -rf ... && ...with a singlerimrafinvocation (or a shared root script).packages/ui/package.json (1)
20-26: Duplicate portability concern for thecleanscriptReplicate the
rimraf-based fix suggested in@plane/typesto ensure Windows compatibility and reduce duplication.packages/services/package.json (1)
8-13: Duplicate portability concern for thecleanscriptAdopt the cross-platform
rimrafapproach described earlier.packages/logger/package.json (1)
16-22: Duplicate portability concern for thecleanscriptApply the same
rimrafrefactor as outlined for@plane/types.packages/shared-state/package.json (1)
10-16:cleanscript not portableSame
rm -rfpattern — adoptrimraf/del-clifor cross-platform safety.packages/decorators/package.json (1)
16-21:cleanscript portability
rm -rfassumes a POSIX shell. Swap inrimrafor similar to avoid breaking Windows usage.
🧹 Nitpick comments (4)
packages/i18n/package.json (2)
10-10: Add--cacheflag to speed up repeated ESLint runs
eslint . --max-warnings 0is correct but will re-analyze the entire tree every time. Adding the cache flag dramatically reduces CI and local turnaround without changing behaviour:-"check:lint": "eslint . --max-warnings 0", +"check:lint": "eslint . --max-warnings 0 --cache",
12-14: Broaden the Prettier glob to cover common source filesJS/JSX and configuration files (
yml,yaml,cjs,mjs) are formatted elsewhere in the repo but are not included here. Omitting them risks inconsistent formatting enforcement between packages.-"check:format": "prettier --check \"**/*.{ts,tsx,md,json,css,scss}\"", -"fix:format": "prettier --write \"**/*.{ts,tsx,md,json,css,scss}\"", +"check:format": "prettier --check \"**/*.{ts,tsx,js,jsx,md,json,yml,yaml,css,scss}\"", +"fix:format": "prettier --write \"**/*.{ts,tsx,js,jsx,md,json,yml,yaml,css,scss}\"",packages/hooks/package.json (1)
16-20: Lint / format commands walkdist/**— expect noise & slow runs
eslint .and the wide Prettier glob will traverse generated output. Either putdistin.eslintignore/.prettierignore, or scope the commands to source files only:"check:lint": "eslint \"src/**/*.{ts,tsx}\" --max-warnings 0", "check:format": "prettier --check \"src/**/*.{ts,tsx,md,json,css,scss}\"",This keeps CI fast and avoids false positives on compiled code.
packages/editor/package.json (1)
27-33: Portable cleaning + tighter quality scope
- Replace
rm -rf …withrimraffor Windows support.- Consider excluding build artefacts (
dist,.turbo, etc.) fromeslint ./ Prettier checks to cut runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/constants/package.json(1 hunks)packages/decorators/package.json(1 hunks)packages/editor/package.json(1 hunks)packages/hooks/.eslintrc.js(0 hunks)packages/hooks/package.json(1 hunks)packages/hooks/tsup.config.ts(1 hunks)packages/i18n/package.json(1 hunks)packages/logger/package.json(1 hunks)packages/propel/package.json(1 hunks)packages/services/package.json(1 hunks)packages/shared-state/package.json(1 hunks)packages/types/package.json(1 hunks)packages/ui/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/hooks/.eslintrc.js
✅ Files skipped from review due to trivial changes (2)
- packages/constants/package.json
- packages/hooks/tsup.config.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/utils/package.json (2)
14-18: Duplicatetsc --noEmitinvocation – keep one source of truth
tsc --noEmitis executed both inbuildand incheck:types. This doubles CI time. Either:
- Drop it from
build(since type errors failcheck:typesanyway), or- Remove the separate
check:typesscript and callnpm run buildinside the turbo task instead.
18-20: **Formatting pattern omits .js / .jsx files
If utility code ever includes plain JS/JSX (e.g. stories, config, tests) they will be left unformatted. Add them unless you intentionally exclude.- "prettier --check \"**/*.{ts,tsx,md,json,css,scss}\"" + "prettier --check \"**/*.{js,jsx,ts,tsx,md,json,css,scss}\""(Apply the same change to
fix:format.)turbo.json (1)
31-54: Disabling cache for every check/fix task forfeits Turbo’s biggest benefit
cache: falseforces these tasks to run on every CI job, even when nothing changed, slowing pipelines. Turbo can safely cache lint/format/type-check by listing appropriate input globs (e.g."inputs": ["**/*.ts","**/*.tsx",".eslintrc.*"]). Consider re-enabling caching with explicit inputs/outputs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/utils/.eslintrc.js(0 hunks)packages/utils/package.json(1 hunks)packages/utils/tsup.config.ts(1 hunks)turbo.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/utils/.eslintrc.js
✅ Files skipped from review due to trivial changes (1)
- packages/utils/tsup.config.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
turbo.json (1)
27-30:devtask no longer depends onbuild– verify DX
RemovingdependsOnmeans runningturbo run devin an app that imports a freshly-cloned package will fail until the package is built once manually. Confirm that all consumer apps can boot on a cold repository.
* feat: adding standard scripts for lint and format check * fix: update packages scripts * fix: adding tsup config to utils package * chore: updated build scripts in logger pacakge
2 participants
Description
Type of Change
Summary by CodeRabbit
.next.