refactor: v2 use workspace platform libraries#23841
Conversation
WalkthroughUpdated apps/api/v2 docs and startup scripts to change local/dev flows and Docker narration; replaced Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
keithwillcode
added
core
![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 (4)
apps/api/v2/README.md (3)
61-61: Clarify Nest/CJS rationale; fix casing.“v2 api … nest.js works only with cjs” is misleading. Nest supports ESM; this project appears to target CJS. Reword for accuracy and casing.
-v2 api can't directly re-use monorepo code because nest.js works only with cjs. Which is why we have dependencies that we first build and watch for changes: +v2 API can't directly reuse monorepo code because this project targets CommonJS. We build dependent packages and watch for changes:
63-63: Fix markdownlint MD014: drop shell prompt symbols.Remove leading “$ ” to satisfy MD014 and improve copy/paste.
-$ yarn local +yarn local-$ yarn dev +yarn dev-$ yarn dev:no-docker +yarn dev:no-dockerAlso applies to: 68-68, 76-76
71-71: Tighten guidance on edits in platform-libraries.Be explicit about when to rerun the local build vs. relying on auto-restart.
-if you make changes to code that v2 uses from platform-libraries or some other manually built dependency changes then wait for v2 to restart. +If you change code in platform-libraries or other manually built deps, let v2 auto‑restart. If outputs look stale, rerun `yarn local` to rebuild.apps/api/v2/package.json (1)
19-21: Guard the new dev flow against missing prebuilds.Since
devno longer runsdev:build, add a lightweight precheck or fallback to prevent “module not found” errors when contributors runyarn devdirectly.- "dev": "ts-node scripts/docker-start.ts && yarn copy-swagger-module && yarn start --watch", + "dev": "node -e \"process.exit(0)\" && ts-node scripts/docker-start.ts && yarn copy-swagger-module && yarn start --watch",If acceptable, replace the no-op with a small pre-script that asserts required workspace build artifacts exist and instructs to run
yarn localotherwise. I can draft that.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
apps/api/v2/README.md(1 hunks)apps/api/v2/package.json(2 hunks)packages/platform/libraries/scripts/local.js(0 hunks)packages/platform/libraries/scripts/postpublish.js(0 hunks)packages/platform/libraries/scripts/prepublish.js(0 hunks)
💤 Files with no reviewable changes (3)
- packages/platform/libraries/scripts/prepublish.js
- packages/platform/libraries/scripts/postpublish.js
- packages/platform/libraries/scripts/local.js
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-02T12:29:32.592Z
Learnt from: CR
PR: calcom/cal.com#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T12:29:32.592Z
Learning: Use `yarn dev` to start the development server
Applied to files:
apps/api/v2/README.md
📚 Learning: 2025-09-02T12:29:32.592Z
Learnt from: CR
PR: calcom/cal.com#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T12:29:32.592Z
Learning: Use `yarn build` to build all packages
Applied to files:
apps/api/v2/README.md
📚 Learning: 2025-09-02T12:29:32.592Z
Learnt from: CR
PR: calcom/cal.com#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T12:29:32.592Z
Learning: Run `yarn type-check` for TypeScript type checking
Applied to files:
apps/api/v2/README.md
📚 Learning: 2025-09-02T12:29:32.592Z
Learnt from: CR
PR: calcom/cal.com#0
File: AGENTS.md:0-0
Timestamp: 2025-09-02T12:29:32.592Z
Learning: Run integration tests with `yarn test <filename> -- --integrationTestsOnly`
Applied to files:
apps/api/v2/README.md
🪛 markdownlint-cli2 (0.17.2)
apps/api/v2/README.md
63-63: Dollar signs used before commands without showing output
(MD014, commands-show-output)
68-68: Dollar signs used before commands without showing output
(MD014, commands-show-output)
76-76: Dollar signs used before commands without showing output
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (5)
apps/api/v2/package.json (3)
19-19: Build order: include platform-libraries earlier if it’s a transitive dep.If other packages consume platform-libraries during their build, put it first to avoid race/failures.
- "dev:build": "yarn workspace @calcom/platform-constants build && yarn workspace @calcom/platform-enums build && yarn workspace @calcom/platform-utils build && yarn workspace @calcom/platform-types build && yarn workspace @calcom/platform-libraries build", + "dev:build": "yarn workspace @calcom/platform-libraries build && yarn workspace @calcom/platform-constants build && yarn workspace @calcom/platform-enums build && yarn workspace @calcom/platform-utils build && yarn workspace @calcom/platform-types build",Confirm actual dependency graph before changing order.
91-113: TypeScript beta may be unstable with Nest/ts-jest.You’re on typescript "^5.9.0-beta". Ensure CI green across ts-jest, Nest CLI, and tooling; pin to a stable if not required.
20-20: Confirmed: scripts/docker-start.ts exists in the repository.
Found at apps/api/v2/scripts/docker-start.ts.apps/api/v2/README.md (2)
73-77: Double-check parity between docker and no-docker dev flows.In apps/api/v2/package.json:
dev= "ts-node scripts/docker-start.ts && yarn copy-swagger-module && yarn start --watch",dev:no-docker= "yarn dev:build && yarn copy-swagger-module && yarn start --watch". Confirm both paths produce equivalent build artifacts and runtime behavior; if they intentionally differ, either make the scripts consistent or document the difference in apps/api/v2/README.md.
66-69: Docs-to-scripts consistency: verify top-levelyarn localexists and builds required workspaces.Automated check failed ("/bin/bash: line 10: /dev/fd/63: No such file or directory") and did not confirm a 'local' script. Verify the repo's top-level package.json defines a "local" script that runs the workspace build/watch steps referenced in the README, and confirm scripts/docker-start.ts (or the equivalent) exists.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
E2E results are ready! |
|
Closing for #23834 |
2 participants
Linear CAL-6009