-
-
Notifications
You must be signed in to change notification settings - Fork 550
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
[i18nIgnore] docs: pnpm install
→ pnpm add
#1324
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
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.
LGTM, thanks for the PR. I do recall this was discussed on last week Talking and Doc'ing
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 catch! Thanks Hideoo!
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.
pnpm add this PR!
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.
This looks good to me!
@HiDeoo any thoughts on doing this across languages and adding an [i18nIgnore]
? Feels like the kind of change that would be safe to do that way and avoid marking more stuff out of date than necessary.
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.
LGTM ✅
Sorry for the late reply. This is a great idea, I don't quite remember if I did not think about it or if I wanted to get an approval before doing it but no matter what, I'll update the branch, update all locales and add the |
* main: (55 commits) [ci] format i18n(es): Update `index` (withastro#1360) [ci] format i18n(fr): Update index (withastro#1367) i18n(es): remove extra section (withastro#1370) i18n(ko-KR): update `index.mdx` (withastro#1363) [ci] format i18n(zh-cn): Update index.mdx (withastro#1361) docs(showcase): add OpenSaaS.sh (withastro#1359) feat(Testimonials): add testimonials to website (withastro#1104) [ci] format [ci] release (withastro#1332) fix: autogenerated sidebar alphabetical sort (withastro#1298) Avoid sidebar scrollbar hiding behind navbar (withastro#1353) Use spawnSync instead of execaSync in `git.ts` (withastro#1347) [ci] format [i18nIgnore] Add src alias (withastro#1322) Italian translation for search.devWarning (withastro#1351) [ci] format i18n(pt-BR): Add translation for `guides/sidebar` (withastro#1346) ...
pnpm install
→ pnpm add
pnpm install
→ pnpm add
Just pushed a new commit applying the changes to all languages and added the There is now only 1 reference to $ rg 'pnpm install'
README.md
16:| `pnpm install` | Installs dependencies | |
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.
Thank you @HiDeoo! This looks good to me. Time to pnpm add
this PR to the docs 🚀
* main: (69 commits) [i18nIgnore] docs: `pnpm install` → `pnpm add` (withastro#1324) [ci] format i18n(zh-cn): Update frontmatter.mdx (withastro#1362) [ci] format i18n(es): Update `index` (withastro#1360) [ci] format i18n(fr): Update index (withastro#1367) i18n(es): remove extra section (withastro#1370) i18n(ko-KR): update `index.mdx` (withastro#1363) [ci] format i18n(zh-cn): Update index.mdx (withastro#1361) docs(showcase): add OpenSaaS.sh (withastro#1359) feat(Testimonials): add testimonials to website (withastro#1104) [ci] format [ci] release (withastro#1332) fix: autogenerated sidebar alphabetical sort (withastro#1298) Avoid sidebar scrollbar hiding behind navbar (withastro#1353) Use spawnSync instead of execaSync in `git.ts` (withastro#1347) [ci] format [i18nIgnore] Add src alias (withastro#1322) ...
* main: (62 commits) [i18nIgnore] docs: `pnpm install` → `pnpm add` (withastro#1324) [ci] format i18n(zh-cn): Update frontmatter.mdx (withastro#1362) [ci] format i18n(es): Update `index` (withastro#1360) [ci] format i18n(fr): Update index (withastro#1367) i18n(es): remove extra section (withastro#1370) i18n(ko-KR): update `index.mdx` (withastro#1363) [ci] format i18n(zh-cn): Update index.mdx (withastro#1361) docs(showcase): add OpenSaaS.sh (withastro#1359) feat(Testimonials): add testimonials to website (withastro#1104) [ci] format [ci] release (withastro#1332) fix: autogenerated sidebar alphabetical sort (withastro#1298) Avoid sidebar scrollbar hiding behind navbar (withastro#1353) Use spawnSync instead of execaSync in `git.ts` (withastro#1347) [ci] format [i18nIgnore] Add src alias (withastro#1322) ...
What kind of changes does this PR include?
Description
As discussed in the last Talking and Doc'ing session, it seems that even tho
pnpm install <pkg>
works and do the expected job, it might be better to align with thepnpm
docs and usepnpm add <pkg>
instead:pnpm install
is used to install all dependencies for a project (reference)pnpm add <pkg>
installs a package and any packages that it depends on (reference)This PR updates the docs to use
pnpm add
instead ofpnpm install
when installing a specific package.If there are any reasons to keep
pnpm install
for reasons that none of the session attendees thought of, we can close this PR altho we might want to notify the docs team so they are aware of it before continuing the same process (https://github.com/withastro/docs/pull/5978/files already includes this changes).