-
Notifications
You must be signed in to change notification settings - Fork 4
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
Rust code generation #66
base: main
Are you sure you want to change the base?
Conversation
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.
Looking good! Just left a few style suggestions and some mild safety ones, but nothing that's a showstopper (though I think anyhow
-> thiserror
is definitely worth doing).
I suppose there are ways we could reduce the boilerplate by storing the string representation, ID and selector in a const array or something similar at the top, but I don't think it's worth it to be stingy with the LOC of a generated file, so this approach looks good to me.
Debug, Hash, PartialEq, Eq, Clone, Copy, serde::Serialize, serde::Deserialize, PartialOrd, Ord, | ||
)] | ||
#[repr(u64)] | ||
pub enum ChainName { |
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.
I may be bikeshedding here, but is this really a ChainName
? I would think this enum would be the Chain
, and that Name
would specifically refer to the stringified version of it.
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.
We inherit the naming, we use this type to read strings from configs or keystore (i.e. actual chain names).
Agree that now might be a good time to align
Co-authored-by: PabloMansanet <pablomansanet@gmail.com>
Co-authored-by: PabloMansanet <pablomansanet@gmail.com>
- use thiserror - reduce `ChainSelector`, `ChainId` types - fn from_chain_selector
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 good to me now! 👍
Introducing Rust code generation.
go templates
rs
folder for rust:chainselectors
with testsNOTES: