-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
ci: add --pnpm flag to correctly publish prerelease
#33688
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
Conversation
|
|
WalkthroughThe Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
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 (1)
.github/workflows/ci.yml (1)
393-394: Clarify the unresolved TODO about./packages/nitro-server.Line 393 introduces a TODO comment, but it remains unresolved. This suggests uncertainty about how to handle
./packages/nitro-server. Given that the PR objectives mention fixing workspace dependency errors for this package, please clarify:
- Should
./packages/nitro-serverbe added to the package paths list on line 394?- Or does the
--pnpmflag handle this package implicitly?- Is this TODO intended to be addressed in a follow-up PR, or should it be resolved as part of this change?
If
nitro-servershould be included, I can help generate the corrected command.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-11T12:34:22.648Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 0
File: :0-0
Timestamp: 2024-11-11T12:34:22.648Z
Learning: Ensure that AI-generated summaries accurately reflect the key changes in the PR, focusing on notable changes such as the removal of unused imports and variables starting with underscores.
Applied to files:
.github/workflows/ci.yml
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
394-394: The--pnpmflag is correctly configured.Based on the documentation, the
--pnpmflag is a valid and documented option forpkg-pr-newthat directs it to usepnpm packinstead of the defaultnpm pack. The flag can be used in workflows asnpx pkg-pr-new publish --pnpm, and your placement between--compactand the package paths is correct.Using
pnpm packinstead ofnpm packis the appropriate choice for this pnpm workspace project, as it properly understands pnpm'sworkspace:*protocol dependencies. This resolves the workspace dependency resolution issue mentioned in the PR objectives.
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
danielroe
left a 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.
indeed!
CodSpeed Performance ReportMerging #33688 will not alter performanceComparing Summary
|
🔗 Linked issue
Related to testing prerelease of #33446
📚 Description
This forces pr.pkg.new to use
pnpm packinstead ofnpm pack.This should fix errors like this:
Should we handle the todo (
# TODO: './packages/nitro-server')?