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

Prepare for NPM publishing, update instructions for NPM and Soldeer #86

Merged
merged 6 commits into from
Dec 4, 2024

Conversation

ericglau
Copy link
Member

@ericglau ericglau commented Nov 26, 2024

  • Updates package.json for publishing to NPM
    • Sets upgrades-core and defender-deploy-client-cli as peerDependencies, since those are often run but not always required (for example if users do not run upgrade safety checks or Defender)
    • Updates the package name to @openzeppelin/foundry-upgrades
      • Users can use the package name as @openzeppelin/foundry-upgrades in their Solidity scripts/tests, but we leave the examples as openzeppelin-foundry-upgrades and use remappings to avoid confusion (since the same instructions are also used for Git submodules or Soldeer)
  • Adds instructions for NPM and Soldeer installations
    • While OpenZeppelin Contracts (and Upgradeable Contracts) could also be installed with NPM or Soldeer, we omit that from the instructions to avoid further confusion, since the steps are already complex

Fixes #40

@ericglau ericglau requested a review from a team November 26, 2024 22:25
@ernestognw
Copy link
Member

If I understand correctly, the optionalDependencies is useful if we actively handle the lack of the dependency (see source). I would consider opening an issue, we can suggest installing them whenever required if they're dynamically imported.

Copy link
Member

@ernestognw ernestognw left a comment

Choose a reason for hiding this comment

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

Not sure how to bypass Snyk's check, perhaps an IT ticket is required. I would consider implement the dependency feedback handling but I wouldn't consider it a blocker.

Thanks

@ericglau
Copy link
Member Author

ericglau commented Dec 4, 2024

Thanks. The intent was to mark those packages as compatible but not always required. They need to be installable when needed though. Changed it to peerDependencies as that more closely reflects this intent.

@ericglau ericglau merged commit 6461ba3 into OpenZeppelin:main Dec 4, 2024
4 checks passed
@ericglau ericglau deleted the npm branch December 4, 2024 21:01
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.

Consider using NPM for distributing this package
2 participants