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

Replace FromVariant with derive_more::from #2308

Open
mversic opened this issue Jun 2, 2022 · 4 comments
Open

Replace FromVariant with derive_more::from #2308

mversic opened this issue Jun 2, 2022 · 4 comments
Labels
good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality

Comments

@mversic
Copy link
Contributor

mversic commented Jun 2, 2022

derive_more::from offers the same functionality as our FromVariant. Considering that we're making more and more use of the derive_more we should remove our custom implementation in favor of derive_more.

TryInto/TryFrom can also be derived to go from enum into one of it's variants

@mversic mversic added good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality labels Jun 2, 2022
@appetrosyan appetrosyan self-assigned this Jun 2, 2022
@appetrosyan
Copy link
Contributor

We need to make sure that we can cover the functionality already present. Since we already have an implementation, the missing functionality can be merged into derive_more.

@appetrosyan
Copy link
Contributor

Example: unboxing JelteF/derive_more#153

@mversic
Copy link
Contributor Author

mversic commented Jun 7, 2022

seems that it's doable with derive_more. If we find derive_more to be too verbose, we can make our own macro wrapper which will delegate to derive_more

@appetrosyan appetrosyan removed their assignment Aug 1, 2022
@ilchu ilchu self-assigned this Feb 16, 2023
@ilchu
Copy link
Contributor

ilchu commented Feb 23, 2023

Currently derive_more doesn't allow returning custom error types from generated TryInto impls, and this breaks our EvaluatesTo type checking. This is going to be fixed in 1.0.0 release, which seems to be happening pretty soon (80% of the milestone completed). So I suggest we revisit this at a later time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST Refactor Improvement to overall code quality
Projects
None yet
Development

No branches or pull requests

3 participants