-
Notifications
You must be signed in to change notification settings - Fork 94
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
[r2r] enum from variant derive macro #1502
Conversation
…erive # Conflicts: # Cargo.lock # mm2src/coins/Cargo.toml # mm2src/coins/utxo.rs
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.
Awesome feature 💯
First review iteration
Signed-off-by: borngraced <samuelonoja970@gmail.com>
Signed-off-by: borngraced <samuelonoja970@gmail.com> Signed-off-by: borngraced <samuelonoja970@gmail.com>
Signed-off-by: borngraced <samuelonoja970@gmail.com> Signed-off-by: borngraced <samuelonoja970@gmail.com>
@sergeyboyko0791 @shamardy this pr is ready for another round of review |
Signed-off-by: borngraced <samiodev@icloud.com>
Signed-off-by: borngraced <samiodev@icloud.com>
Signed-off-by: borngraced <samiodev@icloud.com>
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.
Next review iteration :)
Signed-off-by: borngraced <samiodev@icloud.com>
Signed-off-by: borngraced <samiodev@icloud.com>
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.
Thank you for the fixes!
Next iteration
Signed-off-by: borngraced <samiodev@icloud.com>
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.
Last review notes 🙂
Signed-off-by: borngraced <samiodev@icloud.com>
Signed-off-by: borngraced <samiodev@icloud.com>
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.
A few suggestions only
Signed-off-by: borngraced <samiodev@icloud.com>
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.
One minor non-critical suggestion that may make the code more readable 🙂
Signed-off-by: borngraced <samiodev@icloud.com>
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.
Re-approve!
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.
LGTM!
EnumFromVariant
is very useful for generatingFrom<T>
trait from one enum variant to another. Currently, this crate can only convert enum variant with only some basic inner type such asString
, andEnum
type just like the example below. Can not be used for tuple, struct etc for now.