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

Enhance documentation/ explain how it works #52

Closed
felipecrs opened this issue Jan 25, 2021 · 13 comments
Closed

Enhance documentation/ explain how it works #52

felipecrs opened this issue Jan 25, 2021 · 13 comments

Comments

@felipecrs
Copy link
Contributor

I wonder how it suppresses the node version I've set in my package.json, which is a very good feature, but I wonder how this action does that.

Is it by running volta pin node@version? If so, does it leave the package.json modified?

@rwjblue
Copy link
Collaborator

rwjblue commented Feb 9, 2021

Is it by running volta pin node@version?

Yep!

If so, does it leave the package.json modified?

Yes, it does. This is somewhat intentional, as I'm not sure where the package.json would be reversed. During the full run of the workflow (I suppose until the next run of the volta-cli/action action again) the node/yarn/npm versions specified in the actions config should "override" whatever happens to be in package.json.

@felipecrs
Copy link
Contributor Author

I think this is a feature request for volta instead:

Support running temporary node versions

@rwjblue
Copy link
Collaborator

rwjblue commented Feb 9, 2021

Support running temporary node versions

Hmm, I think this already exists.

Volta run --node=8 npm test

@felipecrs
Copy link
Contributor Author

But it's not suitable for this action for example. It would be something like:

volta use node@8

@rwjblue
Copy link
Collaborator

rwjblue commented Feb 9, 2021

I guess I don't understand what you are asking for. Currently we support two main modes of operation:

  • pinning the project via node-version / yarn-version -- This would be used for the remainder of the action (or until the next invocation of the action changing node-version / yarn-version)
  • using volta run -- This would run a specific command with the associated node / yarn versions; not modifying anything about future command line invocations.

What sort of timing / lifetime are you looking for?

@felipecrs
Copy link
Contributor Author

I'm just looking for something that could be used by this action without changing package.json.

@rwjblue
Copy link
Collaborator

rwjblue commented Feb 9, 2021

Why does the mutation of package.json matter in your case? Is there something that the current volta pin style override system breaks for your setup?

@felipecrs
Copy link
Contributor Author

felipecrs commented Feb 9, 2021

I use semantic-release in most of my projects, and during its execution, it updates the package.json with the new version to release, and commit this change back to GitHub.

In my case, I'm safe, since I don't call semantic-release in the same job which I set custom versions of node with volta.

Nonetheless, I can think of 2 cases where this would break:

  1. For those who don't call semantic-release in a separate job (as advised in their own documentation, it only needs to be called after the npm test, not in other jobs AND the user uses this action to set the node version to run. The semantic-release would commit the newly pinned version of node back to GitHub.

  2. For those who can use this action without pinning their versions in package.json, and still uses semantic-release. The semantic-release would commit the pinned version of node back to GitHub.


Side note: I'm now thinking about doing this deliberately, so I can keep my node and npm versions up-to-date automatically. :)

References volta-cli/volta#905.

@rwjblue
Copy link
Collaborator

rwjblue commented Feb 9, 2021

Thanks for the explanation! I'm not sure there is a reasonable solution we can do here though. I can't think of a good heuristic that would allow us to reset that wouldn't also be annoying for non-release cases.

@felipecrs
Copy link
Contributor Author

felipecrs commented Feb 9, 2021

I thought in something like:

  1. volta set node@8 -> writes the current directory and the desired version of node to $VOLTA_TEMP/set.json
  2. node shim check for $VOLTA_TEMP/set.json before checking for package.json
  3. volta unset node -> removes node from $VOLTA_TEMP/set.json

If you'd like, I can move this discussion to the main volta repo.


But for this repo, I think the current behavior should be documented, so the users would be at least aware of the package.json becoming (git) dirty in their pipelines.

@rwjblue
Copy link
Collaborator

rwjblue commented Feb 9, 2021

But for this repo, I think the current behavior should be documented, so the users would be at least aware of the package.json becoming (git) dirty in their pipelines.

Yes, sounds good to me.

@rwjblue
Copy link
Collaborator

rwjblue commented Feb 9, 2021

RE: the volta internals, I think we'd need volta-cli/volta#282 to be resolved to avoid mutating package.json

@rwjblue
Copy link
Collaborator

rwjblue commented Aug 18, 2022

Going to close this issue for now (thank you for the awesome conversation though @felipecrs), as it seems like a more general feature request over in volta itself.

@rwjblue rwjblue closed this as completed Aug 18, 2022
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

No branches or pull requests

2 participants