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

doc: mention corepack prepare supports dist-tag #44646

Merged
merged 5 commits into from Oct 10, 2022
Merged

Conversation

mrienstra
Copy link
Contributor

corepack prepare yarn@latest --activate is a useful alternative to corepack prepare yarn@x.y.z --activate, added in Corepack v0.120.

Latest pnpm docs (1, 2) mention this option, seems like Node.js Corepack docs should too!

With Node.js v16.17 or newer, you may install the latest version of pnpm by just specifying the tag:

corepack prepare pnpm@latest --activate

Oh, hmm, Node.js v18.6.0 is the oldest version with corepack >= 0.12.0, is that worth mentioning?

(For v16: v16.17.0 is the oldest version with corepack >= 0.12.0)

I can see this that:

<div class="api_metadata">
  <span>Added in: v16.9.0, v14.19.0</span>
</div>

is being added with:

<!-- YAML
added:
  - v16.9.0
  - v14.19.0
-->

Not sure if that can also be used further down to show when dist-tag support was added...

Double oh, I thought I was somehow specifically editing the v18 version of the docs, but now I'm not sure how that works. docs/latest-v16.x/api/corepack & docs/latest-v18.x/api/corepack are identical, and "Options" --> "Edit on GitHub" leads to the same place for both versions.

Ah, OK, I see:
https://github.com/nodejs/node/blob/v18.x/doc/api/corepack.md
https://github.com/nodejs/node/blob/v16.x/doc/api/corepack.md
https://github.com/nodejs/node/blob/v14.x/doc/api/corepack.md (includes "simply", v18 & v16 do not)

Anyhoo, advice on how best to show when dist-tag support was added would be welcome.

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Sep 15, 2022
`corepack prepare yarn@latest --activate` is a useful alternative to
`corepack prepare yarn@x.y.z --activate`
@mrienstra
Copy link
Contributor Author

Note to self: read doc/contributing/pull-requests.md#commit-message-guidelines more carefully next time. 🤦

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

LGTM modulo the linter failure.

doc/api/corepack.md Outdated Show resolved Hide resolved
@aduh95 aduh95 changed the title corepack prepare: dist-tag option doc: mention corepack prepare supports dist-tag Sep 15, 2022
Alternately, a [`dist-tag`](https://docs.npmjs.com/downloading-and-installing-packages-locally#installing-a-package-with-dist-tags) may be used:

```bash
corepack prepare yarn@latest --activate
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
corepack prepare yarn@latest --activate
corepack prepare yarn@stable --activate

@latest is not a valid tag for yarn.

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean? @latest is always a valid tag on npm (it cannot be deleted).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yarn Berry is not on npm:

$ corepack prepare yarn@latest           
(node:96490) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Usage Error: Tag not found (latest)

$ corepack prepare yarn@stable
(node:96513) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
Preparing yarn@stable...

Copy link
Member

Choose a reason for hiding this comment

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

In that case, it would be confusing to point to the npmjs documentation about dist-tags, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, using a link is probably not necessary. And maybe better not to use Yarn in the example since it's the odd one out?

Copy link
Contributor

Choose a reason for hiding this comment

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

We can also have several examples, e.g.:

```bash
corepack prepare pnpm@latest --activate
corepack prepare yarn@stable --activate
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aduh95, I like that ("several examples"), just pushed a commit to change it to that.

FYI, when I run prepare with yarn stable & latest (node v18.9.0, corepack 0.14.0, starting with Yarn 3.2.3), I get:
corepack prepare yarn@stable --activate --> Preparing yarn@stable for immediate activation..., still on Yarn 3.2.3 after.
corepack prepare yarn@latest --activate --> Usage Error: Tag not found (latest), still on Yarn 3.2.3 after (of course).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to say "a tag or range may be used" instead of "a dist-tag may be used", since dist-tag is rarely specified in docs for npm, Yarn or pnpm. I think I was being too specific / low-level. Dropped the link as well.

Tempted to link to https://github.com/npm/node-semver#ranges, that's where /cli/v8/configuring-npm/package-json#dependencies links to.

There aren't any really great high-level docs for tags. /cli/v8/configuring-npm/package-json#dependencies links to /cli/v8/commands/npm-dist-tag, which isn't ideal (focused on modification, not usage), though I guess /cli/v8/commands/npm-dist-tag#purpose is nice. Considering expanding /downloading-and-installing-packages-locally#installing-a-package-with-dist-tags, maybe pulling some content from /cli/v8/commands/npm-dist-tag#purpose. Maybe I'll open an issue for that next week.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tempted to link to https://github.com/npm/node-semver#ranges, that's where /cli/v8/configuring-npm/package-json#dependencies links to.

npm being one of the package managers, I think linking to SemVer rather than to npm would be preferable to not give a competitive adventage to one rather the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aduh95, I'll be out until Monday, feel free to link out to docs for tag and / or range if you think that would be helpful.

Still a little tempted to clarify when Corepack added support for using a tag or range (Corepack v0.120, which shipped with Node.js v18.6.0 and v16.17.0), but not sure if that should be formatted in a special way.

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 16, 2022
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 18, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Sep 18, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/44646
✔  Done loading data for nodejs/node/pull/44646
----------------------------------- PR info ------------------------------------
Title      doc: mention `corepack prepare` supports dist-tag (#44646)
Author     Michael Rienstra  (@mrienstra, first-time contributor)
Branch     mrienstra:patch-1 -> nodejs:main
Labels     doc, author ready
Commits    5
 - doc: corepack prepare: dist-tag option
 - collapsed reference link as per @aduh95
 - docs: several examples, as per @aduh95
 - docs: simplify, just say 'tag or range', drop link
 - docs: corepack: revert ref re-ordering
Committers 1
 - Michael Rienstra 
PR-URL: https://github.com/nodejs/node/pull/44646
Reviewed-By: Antoine du Hamel 
Reviewed-By: Akhil Marsonya 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44646
Reviewed-By: Antoine du Hamel 
Reviewed-By: Akhil Marsonya 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 15 Sep 2022 02:48:48 GMT
   ✔  Approvals: 2
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/44646#pullrequestreview-1110999243
   ✔  - Akhil Marsonya (@marsonya): https://github.com/nodejs/node/pull/44646#pullrequestreview-1111492348
   ✖  Last GitHub CI failed
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3076549334

@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 10, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 10, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/44646
✔  Done loading data for nodejs/node/pull/44646
----------------------------------- PR info ------------------------------------
Title      doc: mention `corepack prepare` supports dist-tag (#44646)
Author     Michael Rienstra  (@mrienstra, first-time contributor)
Branch     mrienstra:patch-1 -> nodejs:main
Labels     doc, author ready
Commits    5
 - doc: corepack prepare: dist-tag option
 - collapsed reference link as per @aduh95
 - docs: several examples, as per @aduh95
 - docs: simplify, just say 'tag or range', drop link
 - docs: corepack: revert ref re-ordering
Committers 1
 - Michael Rienstra 
PR-URL: https://github.com/nodejs/node/pull/44646
Reviewed-By: Antoine du Hamel 
Reviewed-By: Akhil Marsonya 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/44646
Reviewed-By: Antoine du Hamel 
Reviewed-By: Akhil Marsonya 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 15 Sep 2022 02:48:48 GMT
   ✔  Approvals: 2
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/44646#pullrequestreview-1110999243
   ✔  - Akhil Marsonya (@marsonya): https://github.com/nodejs/node/pull/44646#pullrequestreview-1111492348
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
   a2fb3f9785..80270994d6  main       -> origin/main
✔  origin/main is now up-to-date
main is out of sync with origin/main. Mismatched commits:
 - 123babc540 lib: enable global CustomEvent by default
 - 80270994d6 lib: enable global CustomEvent by default
--------------------------------------------------------------------------------
HEAD is now at 80270994d6 lib: enable global CustomEvent by default
   ✔  Reset to origin/main
- Downloading patch for 44646
From https://github.com/nodejs/node
 * branch                  refs/pull/44646/merge -> FETCH_HEAD
✔  Fetched commits as 80270994d6ba..3feef218585d
--------------------------------------------------------------------------------
[main 24fcc4797e] doc: corepack prepare: dist-tag option
 Author: Michael Rienstra 
 Date: Wed Sep 14 18:57:12 2022 -0700
 1 file changed, 6 insertions(+)
[main f31a17dc4f] collapsed reference link as per @aduh95
 Author: Michael Rienstra 
 Date: Thu Sep 15 11:24:50 2022 -0700
 1 file changed, 6 insertions(+), 5 deletions(-)
[main 9339b61aa7] docs: several examples, as per @aduh95
 Author: Michael Rienstra 
 Date: Thu Sep 15 16:22:22 2022 -0700
 1 file changed, 2 insertions(+), 1 deletion(-)
[main 1353268813] docs: simplify, just say 'tag or range', drop link
 Author: Michael Rienstra 
 Date: Thu Sep 15 16:41:50 2022 -0700
 1 file changed, 1 insertion(+), 2 deletions(-)
[main 2568d51925] docs: corepack: revert ref re-ordering
 Author: Michael Rienstra 
 Date: Fri Sep 16 10:18:59 2022 -0700
 1 file changed, 4 insertions(+), 4 deletions(-)
   ✔  Patches applied
There are 5 commits in the PR. Attempting autorebase.
Rebasing (2/10)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: corepack prepare: dist-tag option

corepack prepare yarn@latest --activate is a useful alternative to
corepack prepare yarn@x.y.z --activate

PR-URL: #44646
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Akhil Marsonya akhil.marsonya27@gmail.com

[detached HEAD 5555af4be1] doc: corepack prepare: dist-tag option
Author: Michael Rienstra mrienstra@gmail.com
Date: Wed Sep 14 18:57:12 2022 -0700
1 file changed, 6 insertions(+)
Rebasing (3/10)
Rebasing (4/10)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
collapsed reference link as per @aduh95

Sorted reference links using this pattern:
/^\[[^\w]*(.+?)\]: /

PR-URL: #44646
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Akhil Marsonya akhil.marsonya27@gmail.com

[detached HEAD a3b221903d] collapsed reference link as per @aduh95
Author: Michael Rienstra mrienstra@gmail.com
Date: Thu Sep 15 11:24:50 2022 -0700
1 file changed, 6 insertions(+), 5 deletions(-)
Rebasing (5/10)
Rebasing (6/10)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
docs: several examples, as per @aduh95

PR-URL: #44646
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Akhil Marsonya akhil.marsonya27@gmail.com

[detached HEAD b9c1665e01] docs: several examples, as per @aduh95
Author: Michael Rienstra mrienstra@gmail.com
Date: Thu Sep 15 16:22:22 2022 -0700
1 file changed, 2 insertions(+), 1 deletion(-)
Rebasing (7/10)
Rebasing (8/10)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
docs: simplify, just say 'tag or range', drop link

PR-URL: #44646
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Akhil Marsonya akhil.marsonya27@gmail.com

[detached HEAD 9a74a782ef] docs: simplify, just say 'tag or range', drop link
Author: Michael Rienstra mrienstra@gmail.com
Date: Thu Sep 15 16:41:50 2022 -0700
1 file changed, 1 insertion(+), 2 deletions(-)
Rebasing (9/10)
Rebasing (10/10)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
docs: corepack: revert ref re-ordering

PR-URL: #44646
Reviewed-By: Antoine du Hamel duhamelantoine1995@gmail.com
Reviewed-By: Akhil Marsonya akhil.marsonya27@gmail.com

[detached HEAD 60f39cbd72] docs: corepack: revert ref re-ordering
Author: Michael Rienstra mrienstra@gmail.com
Date: Fri Sep 16 10:18:59 2022 -0700
1 file changed, 4 insertions(+), 4 deletions(-)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/3216159148

@aduh95 aduh95 merged commit 4e659be into nodejs:main Oct 10, 2022
@aduh95
Copy link
Contributor

aduh95 commented Oct 10, 2022

Landed in 4e659be

danielleadams pushed a commit that referenced this pull request Oct 11, 2022
`corepack prepare pnpm@latest --activate` is a useful alternative to
`corepack prepare pnpm@x.y.z --activate`.

PR-URL: #44646
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
@mrienstra mrienstra deleted the patch-1 branch October 19, 2022 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants