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

Updating volta pin to support a --default flag #52

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

charlespierce
Copy link
Contributor

@charlespierce charlespierce commented Feb 22, 2024

Proposal for updating volta pin to add a --default flag that replaces the need for volta install, combining all of the pin operations into a single volta pin command.

Rendered

text/0000-volta-use.md Outdated Show resolved Hide resolved
text/0000-volta-use.md Outdated Show resolved Hide resolved
text/0000-volta-use.md Outdated Show resolved Hide resolved

One alternative would be to only change the verb for `install`, leaving `pin` completely untouched. This would be the least disruptive change, however it's very dependent on the verb / command that we choose. With the proposal of `volta use`, it would feel very unintuitive to run `volta use node@21` in a project, and then immediately get a message that it's set as the global but your local settings override. As a user, I would expect `volta use node@21` to mean the next call to `node --version` to display Node 21 is running. In order to make that work as users expect, we need to unify the two so that `volta use` _does_ actually change the Node for the current context.

Another difficulty of the unified behavior is that it's more difficult to explain when users run into edge cases. Right now there's a clear separation between "local" and "global" with two different commands. Unifying them means we'll need to clearly document how `volta use` behaves in each situation, and how to override that (with the `--global` or `--pin` flags).
Copy link
Contributor

Choose a reason for hiding this comment

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

This is basically exactly my point above. 😂 I take it to be the most significant drawback, and I think it is the thing which really mashes my buttons.

Comment on lines 91 to 97
## `volta default` or `volta global`

In the vein of replacing the verb for `install`, I also considered `default` or `global` for the command. The major issue with those is that they're adjectives, rather than verbs, so it's not immediately clear what the command means. Does `volta default node` mean "choose a default node" or "list the current default node"? That confusion is ultimately why I decided against using either of these for the command.

## `volta set`

Another possibility for the command would be to make it more verbose, e.g. `volta set default node`. That solves the problems around `volta default` or `volta global` by adding a verb to make it very clear what you are doing. However, it makes the commands a fair bit more cumbersome to use, which I feel is ultimately a net negative compared to a single command.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼 to these; I agree quite strongly!

text/0000-volta-use.md Outdated Show resolved Hide resolved
text/0000-volta-use.md Outdated Show resolved Hide resolved
@charlespierce
Copy link
Contributor Author

Suggestion from the main Volta repo: Having volta use be an alias for this combined command would be useful for people switching from / used to nvm, since nvm use is used to change Node versions. It's not exactly the same semantics, so we might want to consider how different it is and how difficult it will be to transition, but I think if we combine the commands it would be a good time to look at making use another alias for the same command.

@chriskrycho
Copy link
Contributor

I definitely don’t have an objection to use, @charlespierce; do you remember why we didn’t choose that in the first place? (I believe that decision preceded my working on the project, but it has been a minute, so one way or another I simply do not recall.)

@charlespierce charlespierce changed the title 'volta use' to replace 'install' and 'pin' Updating volta pin to support a --default flag Oct 3, 2024
@charlespierce
Copy link
Contributor Author

Updated to a new proposal of continuing to use volta pin with an explicit flag (currently --default) for pinning the default version.

@rwjblue
Copy link
Contributor

rwjblue commented Oct 3, 2024

@charlespierce - I really like this last round of updates, definitely makes more sense to me this way.

@charlespierce
Copy link
Contributor Author

I definitely don’t have an objection to use, @charlespierce; do you remember why we didn’t choose that in the first place? (I believe that decision preceded my working on the project, but it has been a minute, so one way or another I simply do not recall.)

I don't have a deep recollection, but I think we actually did have notion use as a command very early on (that's how long ago it was, prior to the rename). I can't find the git history of why we removed it, likely because it caused more confusion due to the differences with nvm

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

Successfully merging this pull request may close these issues.

3 participants