-
Notifications
You must be signed in to change notification settings - Fork 75
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
Doc: clarify which packages are dev dependencies #1022
Conversation
…uctions for yarn. I've proposed splitting the runtime vs dev dependencies into separate lines as aligned with best practices. Additionally, I've added a default-collapsed section for yarn users.
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.
Thanks for the PR, David!
We want to keep the quickstart short, but one extra line to show which dependencies are built-time only is worth it 👍
I love the use of collapsed sections, but I'm afraid we'd rather keep it minimal, and only use npm
. It's not that we don't like Yarn, but there's also pnpm
, bun
, and jsr
.
per bufbuild#1022 (review) revert the sections leaving only the split between runtime/dev deps for npm
Thank you for the review @timostamm. I've reverted the yarn additions. This leaves the runtime/dev split updates and updates the sentence before it to assert the correct ordering of each. |
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.
Pushed up 0e7ae1b to make the same change in two other markdown files.
LGTM!
I've proposed splitting the runtime vs dev dependencies into separate lines as aligned with best practices. Additionally, I've added a default-collapsed section for yarn users.