feat: create new handbook command + test setup for scripts#46
feat: create new handbook command + test setup for scripts#46th0rOdinson merged 19 commits intodevfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. 📝 Walkthrough""" WalkthroughThis change introduces a new CLI tool and supporting infrastructure for creating handbook sites from a template within a monorepo. It adds a Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (15)
scripts/.gitignore (1)
1-5: Minor duplication / path nit
Root.gitignorealready ignores.tmp/; duplicating it here is harmless but redundant.
Also, leading slash confines the rule toscripts/; if you want to ignore all.tmp/folders anywhere, drop the slash:-/.tmp/ +.tmp/scripts/README.md (3)
5-7: Grammar & article nit
Small language fixes improve polish.-`create-handbook.ts`: Create a new handbook site from template. +`create-handbook.ts`: Create a new handbook site from a template. -`copyStaticTo.ts`: Copy shared static assets to a handbook site. +`copyStaticTo.ts`: Copies shared static assets to a handbook site.
10-13: Command may requirerunkeyword
Unlesspnpmhas been configured with a workspace command alias, the typical invocation is
pnpm run create-handbook -- <name>. Worth clarifying to avoid confusion for newcomers.
17-19: Consider adding--recursiveto test example
When running tests scoped to the package,pnpm --filter scripts testworks, but newcomers sometimes forget the recursive flag when at repo root. Maybe show the full command:pnpm -r --filter scripts test.package.json (2)
6-11: Workspace & root-level script wiring look good, but consider avoiding duplicated devDepsAdding the
scriptsworkspace and thecreate-handbookrunner is spot-on.
However,ts-nodeis now declared in both the root andscripts/package.json. In a pnpm workspace the version will be hoisted anyway, so keeping it only once (ideally at the root) reduces future version-drift.No action strictly required, just flagging the duplication.
16-17: Version pinning strategy – double-check future up-keep
prettierandts-nodeare hard-pinned. That’s fine for reproducibility but increases manual maintenance. If that’s unintentional, switch to caret ranges (^3.2.5,^10.9.2) so Dependabot can handle minor/patch bumps automatically.scripts/package.json (1)
15-20: Redundantts-nodedeclaration
ts-nodeis already listed in the rootpackage.json. Unlessscriptsis published independently (it’s marked private), you can drop the duplicate here and rely on the hoisted version.README.md (1)
53-74: Small docs tweak: call-out dependency install stepAfter
pnpm create-handbook my-new-handbookthe new site’s dependencies won’t be installed yet. A quick hint avoids confusion:After creation, you can build and start the new site: +```bash +pnpm install # at repo root to pick up the new workspace +``` ```bash cd sites/my-new-handbook pnpm build:assets pnpm start</blockquote></details> <details> <summary>.github/workflows/test-scripts.yml (1)</summary><blockquote> `41-42`: **Lint: trailing space & missing final newline** Line 42 has a trailing space and the file lacks a newline at EOF, both reported by YAML-lint. Quick fix: ```diff - run: cd scripts && pnpm test␠ + run: cd scripts && pnpm test +scripts/src/copyStaticTo.ts (3)
9-10: Consider making source path configurable.The source path is hardcoded to
packages/common-config/static/common. Consider making this configurable or adding a comment explaining why this specific path is used.// Simple path resolution: scripts/src -> repo root -> packages/common-config/static/common +// TODO: Consider making source path configurable if needed for other use cases const from = path.resolve(__dirname, "..", "..", "packages", "common-config", "static", "common");
14-17: Improve error message clarity.The error message could be more specific about the expected path format (relative to repo root).
if (!to) { - console.error("Please provide a target path: pnpm copy-static <target>"); + console.error("Please provide a target path (relative to repo root): pnpm copy-static <target>"); process.exit(1); }
30-31: Consider using async operations for better performance.The script uses synchronous filesystem operations which could block for large directory copies. Consider using async operations, especially for the copy operation.
// Ensure the target directory exists (create it if it doesn't) -fs.ensureDirSync(toPath); +await fs.ensureDir(toPath);This would require making the script async and using
fs.copy()instead ofcopySync()below.scripts/test-utils/setupTestUtils.ts (1)
39-45: Consider potential race condition in afterEach cleanup.The
afterEachcleanup reads the directory and then removes files in a loop. This could potentially have race conditions if tests are running in parallel or if files are being created during cleanup.Consider using a more atomic approach:
// Clean up after each test afterEach(async () => { - // Clean up any files created during the test - const testFiles = await fs.readdir(TEST_TEMP_DIR).catch(() => []); - for (const file of testFiles) { - await fs.remove(path.join(TEST_TEMP_DIR, file)); - } + // Clean up any files created during the test + try { + await fs.emptyDir(TEST_TEMP_DIR); + } catch (error) { + // Ignore errors if directory doesn't exist + } });scripts/src/create-handbook.ts (1)
21-27: Enhance site name validation for better user experience.The regex validation is correct but could be more restrictive to prevent potential issues with filesystem and web hosting conventions.
Consider adding additional validation:
-// Check for valid characters (alphanumeric, hyphens, underscores) -if (!/^[a-zA-Z0-9_-]+$/.test(siteName)) { +// Check for valid characters and common conventions +if (!/^[a-zA-Z0-9_-]+$/.test(siteName) || siteName.startsWith('-') || siteName.endsWith('-')) { console.error( - "❌ Site name can only contain letters, numbers, hyphens, and underscores" + "❌ Site name can only contain letters, numbers, hyphens, and underscores. Cannot start or end with hyphens." ); process.exit(1); } + +// Prevent reserved names +const reservedNames = ['template', 'test', 'src', 'build', 'node_modules']; +if (reservedNames.includes(siteName.toLowerCase())) { + console.error(`❌ Site name '${siteName}' is reserved. Please choose a different name.`); + process.exit(1); +}scripts/__tests__/create-handbook.test.ts (1)
46-101: Consider splitting the integration test for better isolation.The second test combines multiple concerns (build assets, build site, start server) which makes it harder to debug failures and violates the single responsibility principle for tests.
Split into separate, focused test cases:
describe("create-handbook integration tests", () => { it("builds assets successfully", async () => { // Test only build:assets functionality }); it("builds site successfully", async () => { // Test only build functionality }); it("starts server successfully", async () => { // Test only server startup }); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
.github/workflows/test-scripts.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)package.json(1 hunks)packages/common-config/package.json(1 hunks)packages/common-config/scripts/copyStaticTo.ts(0 hunks)pnpm-workspace.yaml(1 hunks)scripts/.gitignore(1 hunks)scripts/README.md(1 hunks)scripts/__tests__/copyStaticTo.test.ts(1 hunks)scripts/__tests__/create-handbook.test.ts(1 hunks)scripts/jest.config.js(1 hunks)scripts/package.json(1 hunks)scripts/src/copyStaticTo.ts(1 hunks)scripts/src/create-handbook.ts(1 hunks)scripts/test-utils/setupTestUtils.ts(1 hunks)scripts/tsconfig.json(1 hunks)sites/template/package.json(1 hunks)sites/wonderland/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/common-config/scripts/copyStaticTo.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
scripts/__tests__/copyStaticTo.test.ts (1)
scripts/test-utils/setupTestUtils.ts (1)
setupTestFiles(22-26)
🪛 LanguageTool
scripts/README.md
[uncategorized] ~5-~5: Loose punctuation mark.
Context: ...d static assets. - create-handbook.ts: Create a new handbook site from templat...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...ok.ts: Create a new handbook site from template. - copyStaticTo.ts`: Copy shared stati...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~6-~6: Loose punctuation mark.
Context: ... site from template. - copyStaticTo.ts: Copy shared static assets to a handbook...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 GitHub Actions: Test Scripts
scripts/__tests__/create-handbook.test.ts
[error] 71-71: Jest test failure: Expected static assets directory to exist but it does not (expect(received).toBe(expected) failed).
scripts/__tests__/copyStaticTo.test.ts
[error] 31-31: Jest test failure: Expected target directory to be created but it was not (expect(received).toBe(expected) failed).
🪛 actionlint (1.7.7)
.github/workflows/test-scripts.yml
31-31: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-scripts.yml
[error] 42-42: no new line character at the end of file
(new-line-at-end-of-file)
[error] 42-42: trailing spaces
(trailing-spaces)
🔇 Additional comments (14)
.gitignore (1)
14-16: Good call adding a top-level ignore for test artefacts
Helps keep the repo clean and avoids accidental commits of the new Jest tmp dirs.packages/common-config/package.json (1)
11-14: No remaining imports of@common-config/copy-static
I ran the following search across the monorepo (excludingpackages/common-config) and found no references to@common-config/copy-static:rg -n --glob '!packages/common-config/**' '"@common-config/copy-static"' rg -n --glob '!packages/common-config/**' "'@common-config/copy-static'"Since there are no consumer imports or script references, it’s safe to remove the
./copy-staticexport.pnpm-workspace.yaml (1)
2-4: Workspace entry looks fine
Adding"scripts"lets pnpm install / test that package. Nothing else to flag..github/workflows/test-scripts.yml (1)
30-34: Update cache action to v4
actions/cache@v4is the current runner-compatible version; v3 is flagged by actionlint.- - name: Setup pnpm cache - uses: actions/cache@v3 + - name: Setup pnpm cache + uses: actions/cache@v4scripts/tsconfig.json (1)
8-15: TS config looks solidStrict mode, declaration maps and JSON module support are all sensible choices for CLI utilities.
sites/template/package.json (1)
15-15: LGTM! Script command updated correctly.The change properly migrates from the old direct script invocation to using the new centralized scripts package via pnpm filtering.
scripts/__tests__/copyStaticTo.test.ts (1)
51-65: Error handling test looks correct.The CLI error handling test properly validates that the script exits with an error when no target path is provided.
sites/wonderland/package.json (1)
15-15: LGTM! Consistent with template package changes.The script command is correctly updated to use the new centralized scripts package approach, maintaining consistency across handbook sites.
scripts/src/copyStaticTo.ts (2)
1-5: Good documentation explaining script purpose.The comment clearly explains what the script does and why it's needed for Docusaurus static asset handling.
33-39: Error handling and logging look good.The try-catch block with clear success/failure messages and proper exit codes is well implemented.
scripts/test-utils/setupTestUtils.ts (3)
1-10: Well-structured test utility setup.Good documentation and clean separation of concerns with the temp directory path constant.
13-19: Robust error handling in cleanup function.The cleanup function properly handles cases where the directory doesn't exist, preventing test failures from cleanup issues.
22-26: Clean setup function ensures fresh test environment.The setup function correctly ensures a clean state by cleaning up first, then creating the directory.
scripts/jest.config.js (1)
1-25: Well-configured Jest setup for TypeScript CLI testing.The Jest configuration is comprehensive and appropriate for testing TypeScript CLI scripts. Good coverage setup and reasonable timeout configuration.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/test-scripts.yml (1)
44-44: Fix YAML formatting issues.The file has trailing spaces and is missing a newline at the end, which violates YAML best practices.
- - name: Run tests - run: cd scripts && pnpm test + - name: Run tests + run: cd scripts && pnpm test +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.github/workflows/test-scripts.yml(1 hunks)packages/common-config/package.json(2 hunks)scripts/__tests__/copyStaticTo.test.ts(1 hunks)scripts/__tests__/create-handbook.test.ts(1 hunks)scripts/__tests__/setupTestUtils.ts(1 hunks)scripts/jest.config.js(1 hunks)scripts/package.json(1 hunks)scripts/src/copyStaticTo.ts(1 hunks)scripts/src/create-handbook.ts(1 hunks)sites/template/docusaurus.config.ts(2 hunks)sites/template/package.json(1 hunks)sites/wonderland/docusaurus.config.ts(2 hunks)sites/wonderland/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- sites/wonderland/docusaurus.config.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- scripts/tests/copyStaticTo.test.ts
- scripts/package.json
- scripts/src/copyStaticTo.ts
- scripts/jest.config.js
- sites/wonderland/package.json
- sites/template/package.json
- scripts/src/create-handbook.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:63-71
Timestamp: 2025-07-03T19:05:26.826Z
Learning: In monorepo workspaces with pnpm, newly created sites from templates need to have dependencies installed with `pnpm install` in the site directory before running build commands, even if the root workspace has dependencies installed.
scripts/__tests__/create-handbook.test.ts (3)
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:9-17
Timestamp: 2025-07-03T19:02:51.043Z
Learning: In the defi-wonderland/handbook repository, the team prefers to use the `lsof` command for port checking in tests (scripts/__tests__/create-handbook.test.ts) as it's concise and works in their current environments, rather than using Node.js built-in modules for portability.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:63-71
Timestamp: 2025-07-03T19:05:26.826Z
Learning: In monorepo workspaces with pnpm, newly created sites from templates need to have dependencies installed with `pnpm install` in the site directory before running build commands, even if the root workspace has dependencies installed.
packages/common-config/package.json (1)
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
🪛 actionlint (1.7.7)
.github/workflows/test-scripts.yml
33-33: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/test-scripts.yml
[error] 44-44: no new line character at the end of file
(new-line-at-end-of-file)
[error] 44-44: trailing spaces
(trailing-spaces)
🔇 Additional comments (7)
scripts/__tests__/setupTestUtils.ts (1)
1-45: Excellent test utility implementation.The setup utility is well-structured with proper error handling, cleanup logic, and Jest lifecycle management. The separation of concerns between setup and cleanup functions makes it reusable and maintainable.
scripts/__tests__/create-handbook.test.ts (2)
19-38: Excellent server readiness verification implementation.The retry-based approach for server readiness checking is much more reliable than the previous fixed delay approach. This will prevent flaky tests caused by variable server startup times.
9-17: Port checking implementation aligns with team preferences.The
lsofcommand usage for port checking is concise and works well in the current testing environments as preferred by the team.sites/template/docusaurus.config.ts (1)
6-6: LGTM - Import path updated correctly.The import path has been properly updated to reflect the package rename from
@common-configto@handbook/common-config.packages/common-config/package.json (3)
2-2: LGTM - Package name updated consistently.The package name has been updated from
@common-configto@handbook/common-configto follow the established naming convention.
14-17: LGTM - Export structure follows Node.js standards.The new export entry correctly provides both TypeScript types and JavaScript module paths, following proper Node.js module resolution standards.
2-2: Manual confirmation required: ensure all references to@common-confighave been replacedI ran searches across
package.json,.ts,.js,.tsxand.jsxfiles for@common-configand saw no matches—but absence of hits isn’t definitive. Please double-check that no dependent package, config file, CI workflow or documentation still imports or references the old name:
- Imports or requires in any
.ts,.js,.tsx,.jsx- Entries in
package.json, other JSON/YAML config files- References in Markdown docs or CI (
*.md,*.yml,*.yaml)- Environment or custom config files (
.env,*.config.js, etc.)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/test-scripts.yml (3)
27-31: Quote file-descriptor variable to prevent word-splittingA missing pair of quotes around
$GITHUB_ENVcan break the assignment if the path ever contains spaces (rare but possible on self-hosted runners).- echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV + echo "STORE_PATH=$(pnpm store path --silent)" >> "$GITHUB_ENV"
40-41: Ensure deterministic installs with--frozen-lockfileRunning
pnpm installwithout the flag allows silent lock-file changes; CI should fail fast ifpnpm-lock.yamlandpackage.jsondrift.- - name: Install dependencies - run: pnpm install + - name: Install dependencies + run: pnpm install --frozen-lockfile
43-44: Fix lint errors & preferworking-directoryovercdYAML-lint flags trailing spaces on line 44 and missing EOF newline.
You can also drop the manualcdby using the built-in key for clarity.- - name: Run tests - run: cd scripts && pnpm test + - name: Run tests + working-directory: scripts + run: pnpm test +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/test-scripts.yml(1 hunks)scripts/__tests__/copyStaticTo.test.ts(1 hunks)scripts/__tests__/create-handbook.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/tests/copyStaticTo.test.ts
- scripts/tests/create-handbook.test.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:63-71
Timestamp: 2025-07-03T19:05:26.826Z
Learning: In monorepo workspaces with pnpm, newly created sites from templates need to have dependencies installed with `pnpm install` in the site directory before running build commands, even if the root workspace has dependencies installed.
.github/workflows/test-scripts.yml (1)
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
🪛 YAMLlint (1.37.1)
.github/workflows/test-scripts.yml
[error] 44-44: no new line character at the end of file
(new-line-at-end-of-file)
[error] 44-44: trailing spaces
(trailing-spaces)
0xnigir1
left a comment
0xnigir1
left a comment
There was a problem hiding this comment.
one more comment and it's good to go
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/test-scripts.yml (3)
22-26: Pin the pnpm version to avoid future breakages
Usinglatestmeans every workflow run can silently pick up a new major of pnpm, potentially breaking the lock-file checksum or CLI flags. Prefer locking to the current major (e.g.8) or even a full semver + commit SHA.- with: - version: latest + with: + version: 8 # keep in sync with .nvmrc / engines
27-39: Leverage built-in cache frompnpm/action-setupinstead of hand-rolled logic
pnpm/action-setup@v2already offerscache: 'pnpm', which is simpler and less error-prone than manually exportingSTORE_PATHand wiringactions/cache. It also handles path creation edge-cases for you.- - name: Install pnpm - uses: pnpm/action-setup@v2 - with: - version: latest - - - name: Get pnpm store directory - shell: bash - run: | - echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV - - - name: Setup pnpm cache - uses: actions/cache@v4 - with: - path: ${{ env.STORE_PATH }} - key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} - restore-keys: | - ${{ runner.os }}-pnpm-store- + - name: Install pnpm + uses: pnpm/action-setup@v2 + with: + version: 8 + cache: pnpm
44-45: Trim trailing whitespace & add final newline
Line 45 has trailing spaces and the file lacks a newline at EOF, both flagged by yamllint.- run: pnpm test␠ + run: pnpm test +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test-scripts.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:63-71
Timestamp: 2025-07-03T19:05:26.826Z
Learning: In monorepo workspaces with pnpm, newly created sites from templates need to have dependencies installed with `pnpm install` in the site directory before running build commands, even if the root workspace has dependencies installed.
.github/workflows/test-scripts.yml (2)
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:63-71
Timestamp: 2025-07-03T19:05:26.826Z
Learning: In monorepo workspaces with pnpm, newly created sites from templates need to have dependencies installed with `pnpm install` in the site directory before running build commands, even if the root workspace has dependencies installed.
🪛 YAMLlint (1.37.1)
.github/workflows/test-scripts.yml
[error] 45-45: no new line character at the end of file
(new-line-at-end-of-file)
[error] 45-45: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/test-scripts.yml (1)
41-41: Nice catch on--frozen-lockfile
This guarantees deterministic installs in CI. 👍
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/test-scripts.yml (2)
22-27: Drop the explicit cache inpnpm/action-setup; rely onsetup-node’s built-in caching instead.
actions/setup-node@v4already supportscache: "pnpm"(and is executed earlier), so the additional cache toggle here is redundant. Removing it simplifies the workflow and avoids maintaining two separate cache keys.- - name: Install pnpm - uses: pnpm/action-setup@v2 - with: - version: 8 - cache: pnpm + - name: Install pnpm + uses: pnpm/action-setup@v2 + with: + version: 8
28-33: Keep CI deterministic by adding--frozen-lockfile; strip the trailing space and ensure EOF newline.
- You already use
--frozen-lockfile(✅), good.- There’s an extra space at the end of line 33, and the file misses a terminating newline – both pointed out by YAMLlint. Fixing them prevents noisy lint failures.
- run: pnpm test + run: pnpm test +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test-scripts.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:63-71
Timestamp: 2025-07-03T19:05:26.826Z
Learning: In monorepo workspaces with pnpm, newly created sites from templates need to have dependencies installed with `pnpm install` in the site directory before running build commands, even if the root workspace has dependencies installed.
.github/workflows/test-scripts.yml (2)
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/src/create-handbook.ts:0-0
Timestamp: 2025-07-03T19:07:42.089Z
Learning: In the defi-wonderland/handbook repository, the `sites/template` directory is a repository structure that's guaranteed to have a package.json file, so validation checks for package.json existence are not needed in the create-handbook script.
Learnt from: th0rOdinson
PR: defi-wonderland/handbook#46
File: scripts/__tests__/create-handbook.test.ts:63-71
Timestamp: 2025-07-03T19:05:26.826Z
Learning: In monorepo workspaces with pnpm, newly created sites from templates need to have dependencies installed with `pnpm install` in the site directory before running build commands, even if the root workspace has dependencies installed.
🪛 YAMLlint (1.37.1)
.github/workflows/test-scripts.yml
[error] 33-33: no new line character at the end of file
(new-line-at-end-of-file)
[error] 33-33: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
.github/workflows/test-scripts.yml (1)
31-33: Consider running tests from the repo root to cover all workspaces.Executing tests only inside
scripts/means packages in other workspaces (e.g.,sites/*) aren’t validated. If that’s intentional, fine – otherwise switch to the root so every workspace with atestscript is exercised:- working-directory: scripts - run: pnpm test + run: pnpm -r test --if-presentThis leverages pnpm’s recursive mode and respects
--if-presentso workspaces without atestscript are skipped.
3 participants
Implemented a new repository for handling scripts. In this new repo, a script for creating a new handbook was also added!
Issue CHA-330 https://linear.app/defi-wonderland/issue/CHA-330/template-for-new-handbooks
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores