-
Notifications
You must be signed in to change notification settings - Fork 924
Migrate derivative to educe #8125
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
base: unstable
Are you sure you want to change the base?
Conversation
|
It seems this was fixed in recent versions of Aside from that, trying to figure out / reproduce locally the ci build errors on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but for consistency I think we may as well bump superstruct
so we can get rid of the strings in some of the bounds. I've opened a PR here -> #8133
Bump `superstruct` to the latest release `0.10.0`. This version uses a later version of `darling` which is helpful for #8125 Co-Authored-By: Mac L <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jchavarri We have bumped the unstable
branch to superstruct
version 0.10.0
Can you change the trait bounds to remove those strings?
@macladson done! seems to work fine 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me! Just some minor comments
This reverts commit 10108bc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After some internal discussion I've reverted the Default impl change since it's cleaner and is irrelevant for the tests. Sorry for the mix-up.
This PR looks good now so we can merge soon 🚀
Issue Addressed
Fixes #7001.
Proposed Changes
Mostly mechanical replacement of
derivative
attributes witheduce
ones.Attribute Syntax Changes
Note: Some bounds use strings (
bound("E: EthSpec")
) for superstruct compatibility (expected ','
errors).Additional Info
derivate
remains as a transitive dep throughalloy-primitives
->ruint
->ark-ff
->derivative
.