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

Add chalk to dependencies #463

Merged
merged 1 commit into from
Dec 2, 2021
Merged

Add chalk to dependencies #463

merged 1 commit into from
Dec 2, 2021

Conversation

kevinji
Copy link
Contributor

@kevinji kevinji commented Dec 2, 2021

These packages were implicitly pulling in chalk (via @openzeppelin/upgrades-core); this makes the dependency more explicit.

@Amxx
Copy link
Contributor

Amxx commented Dec 2, 2021

Hello @kevinji

As you pointed out, chalk is already in the dependencies of @openzeppelin/upgrades-core. Is there any good reason to not rely on dependency chaining and making it explicit? If so, why only chalk and not the other dependencies in upgrades-core?

Unless there is a strong reason to add it, I'd prefer to putting superfluous things in the package.json

@frangio
Copy link
Contributor

frangio commented Dec 2, 2021

Chalk is used directly by hardhat-upgrades and truffle-upgrades so I agree with including it as a dependency.

Thanks @kevinji!

@frangio frangio closed this Dec 2, 2021
@frangio frangio reopened this Dec 2, 2021
@frangio frangio enabled auto-merge (squash) December 2, 2021 17:25
@frangio frangio merged commit bbe09e5 into OpenZeppelin:master Dec 2, 2021
@kevinji kevinji deleted the add-chalk-to-deps branch December 2, 2021 17:39
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.

3 participants