Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @valebearzotti, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the local development experience for Cloudflare Workers projects within the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Rspack Bundle Analysis
Main CompilerSource:
Total emitted JS: 6.31 MB Runtime CompilerSource:
Total emitted JS: 5.34 MB Package Footprint (npm pack + npm install)
|
There was a problem hiding this comment.
Code Review
This pull request updates the Cloudflare Workers development setup to automatically rebuild the dev server on file changes. The changes look good overall, enabling a much better developer experience. I've left a few comments, mainly regarding cross-platform compatibility for running concurrent scripts and a leftover debug statement. There's also a suggestion to improve the clarity of the updated README file.
I am having trouble creating individual review comments. Click here to see my feedback.
packages/create-xmcp-app/src/helpers/cloudflare.ts (91)
The & operator for running commands concurrently is not cross-platform compatible and will not work on Windows. To ensure generated projects work on all major operating systems, please use a tool like concurrently. This would also require adding concurrently to the devDependencies of the generated package.json.
packageJson.scripts.dev = "concurrently \"xmcp dev --cf\" \"npx wrangler dev\"";
examples/cloudflare-workers/README.md (34-39)
This code block is redundant since pnpm dev already runs wrangler dev, which is what pnpm preview does. This might confuse users. It's better to remove this block and merge the curl commands into a single block under the "Test with curl" heading.
examples/cloudflare-workers/package.json (9)
The & operator for running commands concurrently is not cross-platform compatible (it won't work on Windows out of the box). For better compatibility, consider using a tool like concurrently. You would need to add concurrently as a dev dependency (pnpm add -D concurrently).
"dev": "concurrently \"xmcp dev --cf\" \"npx wrangler dev\",examples/cloudflare-workers/src/tools/hello.ts (19-20)
This console.log appears to be a leftover debugging statement and should likely be removed before merging.
Greptile Overview
|
| Filename | Overview |
|---|---|
| examples/cloudflare-workers/src/tools/hello.ts | Added debug console.log statement that should be removed |
| examples/cloudflare-workers/package.json | Updated dev script to rebuild worker on changes; wrangler bumped to 4.62.0 |
| packages/xmcp/src/compiler/index.ts | Added automatic Cloudflare worker rebuild during dev mode watch cycle |
| "scripts": { | ||
| "build": "xmcp build --cf", | ||
| "dev": "xmcp dev", | ||
| "dev": "xmcp dev --cf & npx wrangler dev", |
There was a problem hiding this comment.
& runs processes in background on Unix but doesn't work on Windows. This will cause the dev script to fail on Windows systems. Use a cross-platform solution like concurrently or npm-run-all:
"dev": "concurrently \"xmcp dev --cf\" \"npx wrangler dev\""Or with npm-run-all:
"dev": "run-p \"dev:*\"",
"dev:xmcp": "xmcp dev --cf",
"dev:wrangler": "npx wrangler dev"Prompt To Fix With AI
This is a comment left during a code review.
Path: examples/cloudflare-workers/package.json
Line: 9:9
Comment:
`&` runs processes in background on Unix but doesn't work on Windows. This will cause the dev script to fail on Windows systems. Use a cross-platform solution like `concurrently` or `npm-run-all`:
```json
"dev": "concurrently \"xmcp dev --cf\" \"npx wrangler dev\""
```
Or with npm-run-all:
```json
"dev": "run-p \"dev:*\"",
"dev:xmcp": "xmcp dev --cf",
"dev:wrangler": "npx wrangler dev"
```
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: examples/cloudflare-workers/src/tools/hello.ts
Line: 19:19
Comment:
debug statement left in code
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb9e14d32f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!packageJson.scripts.dev || packageJson.scripts.dev === "xmcp dev") { | ||
| packageJson.scripts.dev = "xmcp dev --cf & npx wrangler dev"; | ||
| } |
There was a problem hiding this comment.
Avoid backgrounding xmcp dev in npm script
Using xmcp dev --cf & npx wrangler dev is shell-specific: in Windows' default npm shell (cmd.exe), & is just a command separator, so xmcp dev --cf blocks forever and wrangler dev never starts. On POSIX shells, xmcp dev is backgrounded, so Ctrl+C only stops wrangler dev and leaves the xmcp watcher running. This makes pnpm dev unreliable or hard to stop depending on the environment. Consider using a cross-platform process runner (e.g., concurrently) or a script that properly manages both processes.
Useful? React with 👍 / 👎.
No description provided.