Skip to content

Make url in error message clickable link #67

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.

Sign up for GitHub

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

Closed
zakuro9715 wants to merge 1 commit into from
Closed

Make url in error message clickable link #67

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/install-pnpm/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Otherwise, please specify the pnpm version in the action configuration.`)
throw new Error(`No pnpm version is specified.
Please specify it by one of the following ways:
- in the GitHub Action config with the key "version"
- in the package.json with the key "packageManager" (See https://nodejs.org/api/corepack.html)`)
- in the package.json with the key "packageManager" ( See https://nodejs.org/api/corepack.html )`)
Copy link
Collaborator

@KSXGitHub KSXGitHub Nov 8, 2022

Choose a reason for hiding this comment

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

Why is this link even necessary? It is not pnpm's responsibility to teach its user about the fundamental of Node.js.

Suggested change
- in the package.json with the key "packageManager" ( See https://nodejs.org/api/corepack.html )`)
- in the package.json with the key "packageManager"`)

Copy link
Contributor Author

@zakuro9715 zakuro9715 Nov 8, 2022

Choose a reason for hiding this comment

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

If maintainers don't not consider it necessary, it can be removed.

I think that this is a helpful info for users so it is better to keep.
FYI, same link exists in README .

Do I remove it?

Copy link
Collaborator

@KSXGitHub KSXGitHub Nov 8, 2022

Choose a reason for hiding this comment

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

What do you think @zkochan?

Copy link
Contributor Author

@zakuro9715 zakuro9715 Nov 8, 2022

Choose a reason for hiding this comment

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

I created another PR(#69). Merge the one you chose and close the other.

}

if (!packageManager.startsWith('pnpm@')) {
Expand Down