-
-
Notifications
You must be signed in to change notification settings - Fork 91
Correct workspace: protocol usage
#556
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
Summary of ChangesHello @moreal, 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 enhances the monorepo's dependency management by standardizing the use of 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
|
46874b6 to
d699bd7
Compare
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.
Code Review
This pull request correctly addresses invalid workspace: protocol specifiers in package.json files by updating them to the valid workspace:^ format. It also introduces a new CI check in mise.toml to enforce this rule, which is a great addition for maintaining repository health. I have one suggestion to improve the new CI check by making it more efficient and robust.
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
The pnpm workspace: protocol requires a version specifier (*, ^, or ~). Using "workspace:" without a specifier is invalid but pnpm doesn't report a clear error, which can cause issues with tools like moonrepo. This adds a check:manifest:workspace-protocol task that detects invalid patterns and fails the CI check if found. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
d699bd7 to
acd7405
Compare
dahlia
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.
Good job!
Summary
Add a CI check to detect invalid
workspace:protocol specifiers and fix existing invalid usages in the monorepo.Related issue
I couldn't related issues when searching with following queries:
label:integration/fastifyfastifypeerworkspaceChanges
workspace:specifiers toworkspace:^indocs/package.json,examples/*/package.json, andpackages/fastify/package.jsoncheck:manifest:workspace-protocolmise task that uses grep to detectinvalid
workspace:patterns (those without*,^, or~specifier)checktask dependencies so it runs in CIBenefits
The pnpm
workspace:protocol requires a version specifier (*,^, or~).Using
"workspace:"without a specifier is technically invalid, but pnpm doesn't report a clear error.This can cause issues with other tools like moonrepo (e.g., moonrepo) that strictly validate the workspace protocol format.
This change ensures:
Checklist
mise teston your machine?Additional notes
Note for breaking change or bugfix for
@fedify/fastifyAmong the packages affected by this PR, excluding docs and examples, the project that is distributed externally is
@fedify/fastify. Since there was originally no caret, it's unclear how the version should be handled, but when viewed on npmjs.com, it appears to be resolved to a specific version without a caret or tilde, as shown in the quote below.https://www.npmjs.com/package/@fedify/fastify/v/2.0.0-dev.279?activeTab=code
On JSR, it is resolved in a format like
^2.0.0-pr.490.1967+f14078ce, which includes a caret.https://jsr.io/@fedify/fastify@2.0.0-pr.490.1967+f14078ce/dependencies
I think it's common practice to use a caret in
peerDependencies, like^6 || ^7, to indicate compatibility within a certain version range. Therefore, I believe this change is correct. However, there is some discrepancy with the npmjs.com version; since the version range has become broader than before, it doesn't seem to be a breaking change.This pull request will help to complete #555.