refactor: primitive error conversion #327
Conversation
chungquantin
requested review from
evilrobot-01,
Daanvdplas,
al3mart and
peterwht
|
|
||
| [dev-dependencies] | ||
| enum-iterator = "2.1.0" | ||
|
|
||
| [features] | ||
| default = [ "std" ] | ||
| std = [ "codec/std", "scale-info/std" ] | ||
| runtime = [ ] |
There was a problem hiding this comment.
would recommend putting some placeholder in here to make zepter and taplo happy.
| @@ -7,6 +7,8 @@ use codec::{Decode, Encode}; | |||
| use enum_iterator::Sequence; | |||
| #[cfg(feature = "std")] | |||
| use scale_info::TypeInfo; | |||
| #[cfg(feature = "runtime")] | |||
There was a problem hiding this comment.
Drink importing this crate with runtime feature feels odd.
| #[cfg(feature = "runtime")] | |
| #[cfg(all(feature = "runtime", feature = "std"))] |
There was a problem hiding this comment.
Why do we need std? These imports are only required for the conversion, and the conversion is only enabled for runtime feature.
|
|
||
| // Compare all the different `DispatchError` variants with the expected `Error`. | ||
| #[test] | ||
| #[cfg(feature = "runtime")] |
There was a problem hiding this comment.
This should not be required if the "std" feature is added.
There was a problem hiding this comment.
Without runtime feature enabled, the conversion Error::from does not work because From<DispatchError> is only enabled when there is runtime.
| @@ -103,17 +103,9 @@ struct V0Error(pop_primitives::v0::Error); | |||
| impl From<DispatchError> for V0Error { | |||
| fn from(error: DispatchError) -> Self { | |||
There was a problem hiding this comment.
Preferably this should just be:
| fn from(error: DispatchError) -> Self { | |
| fn from(error: DispatchError) -> Self { | |
| Self(error.into()) | |
| } |
Makes me wonder whether to remove that whole DecodingFailed variant as the developer should never receive this error anyway @evilrobot-01 .
|
Forgive me if I am missing something, but if drink is implying/requiring a dependency on a specific runtime, then why can't we just reuse the error conversion defined within the runtime rather than moving into primitives? |
|
With the refactor in r0gue-io/pop-drink#14 this PR is no longer needed. |
Moving the conversion code
DispatchError -> V0Errorforpop_primitives::Errorindevnet/versioning.rstopop_primitivesto handleDispatchError -> pop_primitives::Error.We need this refactor for for the pop-api refactor in example contract to work. For more context, please take a look at:
PSP22Errorconversion & assertion #324.Note: Due to the empty feature
runtimeinpop-primitives, zepter and taplo conflicts