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

Refactor crate determination in new ethers-macro crate #555

Merged

Conversation

sebastinez
Copy link
Contributor

Motivation

Currently when wanting to implement the EIP712 derive macro, one has to import ethers_core as a separate dependency, or declare it like ethers::core as ethers_core.
This is cumbersome and not clean.

Solution

By making use of the convenience function in ethers::contract::abigen::ethers_core_crate() we can compute a path and show it in the derive macro.
By doing this we can avoid this additional ethers_core dependency in external projects.

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@sebastinez
Copy link
Contributor Author

Hey @gakonst, this should solve the issue with needing to import ethers_core as separate dependency.
Let me know if you have any feedback regarding this PR.

@mattsse
Copy link
Collaborator

mattsse commented Nov 4, 2021

this would be an opportunity to think about ethers-macros #494 again.
At first, we could simply create it and move the create detection there and then figure out how to proceed.

@gakonst
Copy link
Owner

gakonst commented Nov 5, 2021

Yeah so: Let's make an ethers-macros crate and pull in the crate detection functions there, and use them in both ethers-contract-abigen and ethers-712. Code changes look good otherwise beyond the dependencies.

@sebastinez
Copy link
Contributor Author

Yeah so: Let's make an ethers-macros crate and pull in the crate detection functions there, and use them in both ethers-contract-abigen and ethers-712. Code changes look good otherwise beyond the dependencies.

Okay no problem!
Will do a push today with these changes

@gakonst
Copy link
Owner

gakonst commented Nov 5, 2021

Amazing. Thank you.

@sebastinez sebastinez force-pushed the sebastinez/eip712-add-ethers-core-path branch from 0efa345 to fe871c4 Compare November 5, 2021 09:13
@sebastinez sebastinez changed the title Compute ethers-core path in derive eip712 Refactor crate determination in new ethers-macro crate Nov 5, 2021
@sebastinez sebastinez force-pushed the sebastinez/eip712-add-ethers-core-path branch from 5908899 to 1c10ffa Compare November 5, 2021 09:25
@@ -14,6 +14,7 @@ rlp = { version = "0.5.0", default-features = false }
ethabi = { version = "15.0", default-features = false, features = ["full-serde", "rlp"] }
arrayvec = { version = "0.7.2", default-features = false }
rlp-derive = { version = "0.1.0", default-features = false }
cargo_metadata = "0.14.0"
Copy link
Owner

Choose a reason for hiding this comment

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

Should this be optional = true and enabled only if macros is enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that makes sense since it's only used by the macros crate for now

@gakonst gakonst merged commit e726362 into gakonst:master Nov 5, 2021
@sebastinez sebastinez mentioned this pull request Nov 5, 2021
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.

None yet

3 participants