Skip to content

feat: clear error on invalid node version#6916

Merged
zkochan merged 8 commits intomainfrom
err-on-invalid-node-version-6909
Aug 9, 2023
Merged

feat: clear error on invalid node version#6916
zkochan merged 8 commits intomainfrom
err-on-invalid-node-version-6909

Conversation

@KSXGitHub
Copy link
Copy Markdown
Contributor

related to #6909

@KSXGitHub KSXGitHub requested a review from zkochan as a code owner August 7, 2023 07:51
Comment thread env/node.fetcher/src/index.ts Outdated
Comment thread env/node.fetcher/src/index.ts Outdated
@zkochan
Copy link
Copy Markdown
Member

zkochan commented Aug 7, 2023

The tests have failed.

@KSXGitHub

This comment was marked as resolved.

@KSXGitHub

This comment was marked as resolved.

@KSXGitHub
Copy link
Copy Markdown
Contributor Author

KSXGitHub commented Aug 7, 2023

@zkochan I have tested each case to see which one return 404. Any case that return 404 is invalid. All invalid cases should cause an error in the latest commit.

@KSXGitHub KSXGitHub requested a review from zkochan August 7, 2023 12:22
@KSXGitHub
Copy link
Copy Markdown
Contributor Author

KSXGitHub commented Aug 7, 2023

@zkochan So the CI errors was caused by pnpm env use. I'm confused. Why can pnpm env use -g 19 succeed but use-node-version=19 result in failure? Do you prefer use-node-version emit error or be the same as pnpm env use?

@zkochan
Copy link
Copy Markdown
Member

zkochan commented Aug 7, 2023

use-node-version should work only with exact versions. pnpm env use can work with any specifier.

@KSXGitHub
Copy link
Copy Markdown
Contributor Author

I have separated the parser for .npmrc#use-node-version from the parser for pnpm env use.

@zkochan zkochan merged commit 66423df into main Aug 9, 2023
@zkochan zkochan deleted the err-on-invalid-node-version-6909 branch August 9, 2023 22:38
KSXGitHub added a commit to pnpm/pnpm.io that referenced this pull request Aug 10, 2023
zkochan pushed a commit to pnpm/pnpm.io that referenced this pull request Aug 10, 2023
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.

2 participants