-
Notifications
You must be signed in to change notification settings - Fork 8
chore: update build scripts and improve copy-playground script #264
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: develop
Are you sure you want to change the base?
Conversation
…tter path handling
…age component builds
WalkthroughThis pull request refactors the build process by extracting build orchestration logic from GitHub Actions workflows into dedicated Node.js scripts. The workflows are simplified to invoke the new scripts, and package.json is updated to reference them. A --nocomponents flag allows conditional component builds. The package name is changed to "robot-root". Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant GHA as GitHub Actions
participant BuildDocs as build-docs.js
participant Pnpm
participant CopyPlayground as copy-playground.js
User->>GHA: Trigger workflow
GHA->>BuildDocs: Execute build:docs --nocomponents
rect rgb(200, 230, 201)
Note over BuildDocs: Check --nocomponents flag
alt Flag not present
BuildDocs->>Pnpm: pnpm build:components
Pnpm-->>BuildDocs: ✓ complete
end
end
BuildDocs->>Pnpm: pnpm build:playground
Pnpm-->>BuildDocs: ✓ complete
BuildDocs->>Pnpm: pnpm -F docs build
Pnpm-->>BuildDocs: ✓ complete
BuildDocs-->>GHA: ✓ success
GHA->>CopyPlayground: Execute copy-playground.js [source] [dest]
rect rgb(220, 240, 255)
Note over CopyPlayground: Parse & validate arguments
CopyPlayground->>CopyPlayground: Verify source exists
CopyPlayground->>CopyPlayground: Verify dest parent exists
end
CopyPlayground->>CopyPlayground: cpSync(source, dest, recursive)
CopyPlayground-->>GHA: ✓ success
GHA->>GHA: Verify build output
GHA->>GHA: Deploy docs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
scripts/copy-playground.js (1)
2-35: CLI handling and validation look good; consider making log messages path-agnosticThe argument parsing, path resolution, and existence checks are solid for this use case. The only minor nit is that the log lines still hard‑code
playground dist to docs/dist/playground, even though the script now accepts arbitrarysource/dest. If you plan to reuse this script elsewhere, consider logging the actual paths instead, for example:console.log(`Copying from ${source} to ${dest}...`) cpSync(source, dest, { recursive: true, force: true }) console.log(`✓ Successfully copied from ${source} to ${dest}`)scripts/build-docs.js (1)
3-25: Confirm that--nocomponentsis actually reaching this script frompnpm build:docsThe sequencing here (components → playground → docs) and the
skipComponentsflag look good. The workflows invoke this viapnpm build:docs --nocomponents; depending on pnpm’s semantics, flags starting with-may require a--separator to be passed through to the script.If pnpm is not forwarding
--nocomponents, this script will still runpnpm build:components, meaning components get built twice in CI (once viapnpm build, once here). Please double‑check pnpm’srunargument‑forwarding; if needed, update workflows to:pnpm build:docs -- --nocomponents
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/auto-publish.yml(1 hunks).github/workflows/dispatch-publish.yml(1 hunks)package.json(1 hunks)scripts/build-docs.js(1 hunks)scripts/copy-playground.js(1 hunks)
🔇 Additional comments (3)
.github/workflows/dispatch-publish.yml (1)
85-91: Docs step depends on--nocomponentsbeing forwarded toscripts/build-docs.jsThis workflow now calls
pnpm build:docs --nocomponentsafter having already runpnpm build(components). That’s consistent with the intent ofscripts/build-docs.js, which conditionally skips the components build based on--nocomponents.As noted in the script, please verify that pnpm indeed passes
--nocomponentsthrough to thebuild:docsscript; otherwise components will still be rebuilt here. If pnpm requires a separator, this step may need to be:run: pnpm build:docs -- --nocomponentspackage.json (1)
2-11: Docs build/postbuild script wiring looks consistent with the new Node helpersRenaming the root package to
"robot-root"is fine, and the newbuild:docs/postbuild:docsscripts cleanly delegate to the Node helpers introduced inscripts/build-docs.jsandscripts/copy-playground.js. The hard‑coded arguments forpostbuild:docsmatch the current playground/dist layout and the workflow expectations.No issues from a build orchestration standpoint.
.github/workflows/auto-publish.yml (1)
106-112: Same--nocomponentsforwarding concern in Auto Publish workflowThis step correctly reuses
pnpm build:docs --nocomponentsand sets VITEPRESS_BASE / PLAYGROUND_BASE per dist tag, matching the new build-docs script design. As withdispatch-publish.yml, the effectiveness of--nocomponentshinges on pnpm passing the flag through toscripts/build-docs.js.Please confirm pnpm’s behavior here and, if necessary, switch to:
run: pnpm build:docs -- --nocomponentsto ensure the flag is seen by the script and components aren’t rebuilt.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.