Query Response Constructors #1766
Conversation
| @@ -55,6 +58,8 @@ pub struct BalanceResponse { | |||
| pub amount: Coin, | |||
| } | |||
|
|
|||
| impl_response_constructor!(BalanceResponse, amount: Coin); | |||
There was a problem hiding this comment.
Great. This also brings up #1767
packages/std/src/query/mod.rs
Outdated
| #[doc(hidden)] | ||
| #[allow(dead_code)] | ||
| pub fn new($( $field: $t),*) -> Self { | ||
| Self { $( $field: $field.into() ),* } |
There was a problem hiding this comment.
With this into() the constructor arg can have a different type than the field itself? Looks cool. Do you have an example for where they are different?
There was a problem hiding this comment.
Ah, that's actually a leftover from a version where I set all the parameters to be impl Into<$t>.
Keeping the .into() here would allow making some of the arguments impl Into<_> on the caller-side, but not sure if we really need that, since these constructors are mostly for internal use anyways?
I haven't done it in any of the calls here, but e.g. the BondedDenomResponse could have this one:
impl_response_constructor!(BondedDenomResponse, denom: impl Into<String>);There was a problem hiding this comment.
Then let's simplify and remove the .into(). We can always add it back if needed.
| @@ -13,4 +13,4 @@ use serde::de::DeserializeOwned; | |||
| /// - multi-test/cw-sdk: create a default instance and mutate the fields | |||
| /// | |||
| /// This trait is crate-internal and can change any time. | |||
| pub(crate) trait QueryResponseType: Default + DeserializeOwned {} | |||
| pub(crate) trait QueryResponseType: DeserializeOwned {} | |||
There was a problem hiding this comment.
I wonder (maybe not in scope) if this should require a few other things, like Debug, PartialEq and Clone. Certainly helpful for test code and should be implemented for all those types coming from JSON.
There was a problem hiding this comment.
Yes, good idea. I also just noticed that the staking responses don't implement this trait yet (because they don't have Default impls). I guess we want to implement it for them, now that Default is no longer needed.
this is the 1.3 part of #1765
I also added the constructor to the existing responses (but can remove if you prefer)