Skip to content
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] Fix pnpm command typos #5284

Merged
merged 2 commits into from
Nov 5, 2023
Merged

[i18nIgnore] Fix pnpm command typos #5284

merged 2 commits into from
Nov 5, 2023

Conversation

mingjunlu
Copy link
Contributor

@mingjunlu mingjunlu commented Nov 5, 2023

Description

I found that many package installation snippets use pnpm install <pkg> instead of pnpm add <pkg>.

According to pnpm's documentation:

This PR changes those to the latter ✍️

Related issues & labels

  • Suggested label: code snippet update

Copy link

netlify bot commented Nov 5, 2023

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 986b069
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/6547dc2426a9c60007d0d3a9
😎 Deploy Preview https://deploy-preview-5284--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@astrobot-houston astrobot-houston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! Thank you for opening your first PR to Astro’s Docs! 🎉

Here’s what will happen next:

  1. Our GitHub bots will run to check your changes.
    If they spot any broken links you will see some error messages on this PR.
    Don’t hesitate to ask any questions if you’re not sure what these mean!

  2. In a few minutes, you’ll be able to see a preview of your changes on Netlify 🥳

  3. One or more of our maintainers will take a look and may ask you to make changes.
    We try to be responsive, but don’t worry if this takes a few days.

@mingjunlu mingjunlu changed the title Fix pnpm code snippet typos Fix pnpm command typos Nov 5, 2023
@sarah11918 sarah11918 added help - confirm behaviour Walk through the example/issue and confirm this is a general behaviour, or a correct update to make. code snippet update Updates a code sample: typo, outdated code etc. labels Nov 5, 2023
Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, there is a difference between pnpm install and pnpm add when using the pnpm package manager.

So in general:

  • Use pnpm install when setting up a brand new project, or when you want to fully reinstall all dependencies.
  • Use pnpm add when you just want to add a new dependency to an existing project.

The snippets likely use pnpm install because they are showing how to install a package in a generic, standalone way. But in the context of an existing project, pnpm add is preferrable for adding new packages.

Good catch on the distinction! Using the right command for the situation can save time and avoid unnecessary installs.

Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for handling this @mingjunlu, I've left you a couple of suggestions let me know your thoughts.

@@ -71,7 +71,7 @@ Astro must be installed locally, not globally. Make sure you are *not* running `
</Fragment>
<Fragment slot="pnpm">
```shell
pnpm install astro
pnpm add astro
Copy link
Member

@dreyfus92 dreyfus92 Nov 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't replace install here, since this guide is about creating a brand new project.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! Thanks for pointing it out 💡

@@ -656,7 +656,7 @@ If the image is merely decorative (i.e. doesn’t contribute to the understandin
When using a [strict package manager](https://pnpm.io/pnpm-vs-npm#npms-flat-tree) like `pnpm`, you may need to manually install Sharp into your project even though it is an Astro dependency:

```bash
pnpm install sharp
pnpm add sharp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not completely sure about this one, i'd stick with a fresh re install of sharp tho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! Let's keep the original one

@sarah11918
Copy link
Member

Thanks so much @mingjunlu for helping us get the commands just right. And thank you for this excellent review, @dreyfus92 with extra context and for thoughtfully considering each individual change. Go Team! 🙌

I'm going to add the title keyword so that this PR touching multiple files doesn't affect our translation tracking status, and I'm going to apply the suggestions so this can be merged and helping people in docs right away!

@mingjunlu - If you find any error in the published version of this PR, please do make another one and let us know! I do agree with @dreyfus92 's assessment that in some cases, install is preferable, and will make those changes.

Thanks again, and welcome to Team Docs! 🥳

@sarah11918 sarah11918 changed the title Fix pnpm command typos [i18nIgnore] Fix pnpm command typos Nov 5, 2023
@sarah11918 sarah11918 merged commit 0a62d07 into withastro:main Nov 5, 2023
@mingjunlu
Copy link
Contributor Author

Woah! To be honest, I didn't expect to receive response that fast! 🚀
@dreyfus92 @sarah11918 Thank you both for taking care of this PR and providing such valuable feedback. I'm glad that I could help 😊

@mingjunlu mingjunlu deleted the fix/pnpm-command-typos branch November 6, 2023 02:50
yanthomasdev added a commit that referenced this pull request Nov 12, 2023
Update translation with #5284 and #5363

L39 &78

Co-authored-by: Yan Thomas <[email protected]>
dreyfus92 added a commit that referenced this pull request Jan 14, 2024
…mdx` (#6363)

* i18n(fr) Update reading-time.mdx

Just for translator tracker because the PR #5766 should have been [ignore]

* Update astro-pages.mdx

* Update framework-components.mdx

* Update layouts.mdx

* Update routing.mdx

* Update editor-setup.mdx

* Update configuring-astro.mdx

* Update cloudflare.mdx

* Update deno.mdx

* Update netlify.mdx

* Update vercel.mdx

* Update imports.mdx

* Update auto.mdx

* Update manual.mdx

* Update cli-reference.mdx

Update and del some parts

* Update cli-reference.mdx

Update fiule with PR #5512 #5651 #5604 #5765 #6040 #6043 #6109 #6267

* Update auto.mdx

* Update cloudflare.mdx

fix PR #5284

* Update markdown-content.mdx

* Update auto.mdx

* Update src/content/docs/fr/core-concepts/astro-pages.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/astro-pages.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/astro-pages.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/framework-components.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/framework-components.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/guides/markdown-content.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/install/manual.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/install/manual.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/reference/cli-reference.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Apply suggestions from code review

---------

Co-authored-by: Paul Valladares <[email protected]>
ematipico pushed a commit that referenced this pull request Jan 26, 2024
…mdx` (#6363)

* i18n(fr) Update reading-time.mdx

Just for translator tracker because the PR #5766 should have been [ignore]

* Update astro-pages.mdx

* Update framework-components.mdx

* Update layouts.mdx

* Update routing.mdx

* Update editor-setup.mdx

* Update configuring-astro.mdx

* Update cloudflare.mdx

* Update deno.mdx

* Update netlify.mdx

* Update vercel.mdx

* Update imports.mdx

* Update auto.mdx

* Update manual.mdx

* Update cli-reference.mdx

Update and del some parts

* Update cli-reference.mdx

Update fiule with PR #5512 #5651 #5604 #5765 #6040 #6043 #6109 #6267

* Update auto.mdx

* Update cloudflare.mdx

fix PR #5284

* Update markdown-content.mdx

* Update auto.mdx

* Update src/content/docs/fr/core-concepts/astro-pages.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/astro-pages.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/astro-pages.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/framework-components.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/core-concepts/framework-components.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/guides/markdown-content.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/install/manual.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/install/manual.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Update src/content/docs/fr/reference/cli-reference.mdx

Co-authored-by: Paul Valladares <[email protected]>

* Apply suggestions from code review

---------

Co-authored-by: Paul Valladares <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code snippet update Updates a code sample: typo, outdated code etc. help - confirm behaviour Walk through the example/issue and confirm this is a general behaviour, or a correct update to make.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants