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

feat(docs): replace npm@<7 with yarn #1260

Merged
merged 1 commit into from
Sep 23, 2023
Merged

Conversation

gpoole
Copy link
Contributor

@gpoole gpoole commented Sep 11, 2023

What is the purpose of this pull request? (put an "X" next to item)

[x] Documentation update
[ ] Bug fix
[ ] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)

Update the install command in the readme for peer dependencies to quote strings with characters interpreted by shells like bash and zsh.

Which issue (if any) does this pull request address?

The install command as currently given is not usable in zsh without escaping the ^s.

@welcome
Copy link

welcome bot commented Sep 11, 2023

🙌 Thanks for opening this pull request! You're awesome.

@mightyiam
Copy link
Owner

Does anyone still such old npm?

Wouldn't we rather remove this altogether?

@gpoole
Copy link
Contributor Author

gpoole commented Sep 12, 2023

Yeah not sure about that version of NPM. The reason I needed it was for Yarn since (unless I've missed something) it doesn't seem to install peer deps automatically either. How about keeping it and changing it to a Yarn command?

@mightyiam
Copy link
Owner

Really? Yarn behaves this way? Well... soon some day we won't have plugins as peerDeps. So, if anything, I would remove this.

@mightyiam mightyiam closed this Sep 12, 2023
@mightyiam
Copy link
Owner

Sorry, I didn't mean to close. Would you like to remove it?

@mightyiam mightyiam reopened this Sep 12, 2023
@gpoole
Copy link
Contributor Author

gpoole commented Sep 12, 2023

Yes, looks like this is Yarn's chosen behaviour for the present anyway @mightyiam: yarnpkg/yarn#1503 (comment)

I do think it's useful for Yarn users right now even if you're planning to remove it later, so would you consider keeping it in Yarn form until the time peer deps aren't required at all? I think removing it entirely will make life harder for Yarn users, since we'd have to figure out the install command for the peer deps from scratch instead of just modifying the handy npm command you have there, so if you'd rather not switch it to a Yarn command I'd say I'd definitely vote for leaving it alone.

@mightyiam
Copy link
Owner

Okay, so we should replace the existing section with a yarn specific one.

@gpoole
Copy link
Contributor Author

gpoole commented Sep 12, 2023

Updated with a Yarn sample, does that look ok?

readme.md Outdated
eslint-config-standard-with-typescript@latest
eslint-config-standard-with-typescript
Copy link
Owner

Choose a reason for hiding this comment

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

Is this change related?

Copy link
Contributor Author

@gpoole gpoole Sep 13, 2023

Choose a reason for hiding this comment

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

If you specify a version explicitly, yarn add will put the literal version specified into your package.json, unlike npm which will resolve latest to a specific patch version range and write that. Leaving the version specifier off will implicitly request latest, so the result is the same.

readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
@mightyiam
Copy link
Owner

Seems like a test needs to be amended.

closes mightyiam#1254

Co-authored-by: jay-bulk <jay.bulk@proton.me>
Co-authored-by: Shahar "Dawn" Or <mightyiampresence@gmail.com>
Co-authored-by: Greg Poole <gpoole@users.noreply.github.com>
@mightyiam mightyiam changed the title docs: zsh-friendly install command for npm@<7 feat(docs): replace npm@<7 with yarn Sep 23, 2023
@rhettjay rhettjay merged commit 54f2883 into mightyiam:master Sep 23, 2023
6 checks passed
@welcome
Copy link

welcome bot commented Sep 23, 2023

🎉 Congrats on getting your first pull request landed!

@eslint-config-love-release-bot

🎉 This PR is included in version 39.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants