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

feat: add options for package manager #3328

Merged
merged 13 commits into from
Oct 27, 2024

Conversation

arihantbansal
Copy link
Contributor

@arihantbansal arihantbansal commented Oct 23, 2024

  • Changes current behavior (require warn, try yarn, warn if yarn is missing, use npm) to using specified package manager (npm, yarn, pnpm) with npm as default fallback if not specified or if specified package manager is missing
  • Use package manager option in test scripts too
  • Update 'mismatched version' error message to also use specified package manager
    - Adds the package_manager flag in init, build, test, verify, and publish commands.

This PR doesn't change yarn being used to build Anchor itself.

This PR is inspired by #2934, and builds up on it.

Addresses #2881.

Copy link

vercel bot commented Oct 23, 2024

@arihantbansal is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

This is a great addition, but I think it would be much better if the package manager could be controlled from Anchor.toml rather than having to pass CLI arguments each time, since this is not something people would want to change too often. For example, we could have something like:

[toolchain]
# Possible values "npm", "yarn" or "pnpm"
package_manager = "npm"

One place where it makes sense to have a CLI option for this is with the init command (because Anchor.toml hasn't been created yet).

cli/src/checks.rs Outdated Show resolved Hide resolved
cli/src/config.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
cli/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Almost ready. Could you also note this feature in the CHANGELOG?

cli/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@acheroncrypto acheroncrypto left a comment

Choose a reason for hiding this comment

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

Adding a dedicated field for the package manager was already on my to-do list (within a greater refactoring of Anchor.toml), but no reason not to have it now. Thanks!

We'll most likely make npm the default in v0.32 as mentioned earlier.

@acheroncrypto acheroncrypto merged commit d34c04a into coral-xyz:master Oct 27, 2024
49 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants