Skip to content
This repository was archived by the owner on Oct 19, 2024. It is now read-only.

Conversation

@dbelv
Copy link
Contributor

@dbelv dbelv commented May 9, 2023

Motivation

Closes #2408

Solution

Remove feature flag enabling by default.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@dbelv
Copy link
Contributor Author

dbelv commented May 9, 2023

Muscle memory had me doing cargo fmt which is unfortunately why large amount of touched files within the same commit vs the actual implementation of the issue.

Forgot to set nightly - it was reverted with the next commit

Copy link
Collaborator

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Can you keep the feature just in the Cargo.toml, to avoid breakage for now? It will be removed in the next major bump (e.g. for '3.0').

@dbelv dbelv marked this pull request as ready for review May 9, 2023 14:31
@dbelv
Copy link
Contributor Author

dbelv commented May 9, 2023

Sorry I have overloaded the PR for clippy fixes as well

openssl = ["ethers-contract-abigen/openssl"]

# Deprecated
eip712 = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add them back here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah right, yep.

macros = ["syn", "cargo_metadata", "once_cell"]

# Deprecated
eip712 = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

Copy link
Collaborator

@prestwich prestwich left a comment

Choose a reason for hiding this comment

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

keep the flag in the Cargo.toml. Removing it will break compilation for library consumers


## Feature flags

- `eip712`: Provides the `Eip712` trait and derive procedural macro for EIP-712 encoding of typed data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

modify to say "does nothing"

}
}

#[cfg(feature = "eip712")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

combine with main impl block

@dbelv dbelv changed the title Remove the eip712 feature flag and have it enabled by default. Fmt files Remove the eip712 feature flag and have it enabled by default May 10, 2023
Copy link
Collaborator

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Thanks!

@DaniPopes DaniPopes merged commit 34f6a8c into gakonst:master May 10, 2023
@dbelv dbelv deleted the eip712-remove branch July 16, 2023 23:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove eip712 feature

3 participants