DispatchError::Module is now a tuple variant in latest Substrate #439
Conversation
|
CI's angry because of outdated metadata? |
|
I'll see whether it succeeds now; I'm assuming it'll start passing when a new binary is built with the breaking changes in. (although we haven't had nightly CI complain yet so maybe not just yet!) |
|
I need this 🙏 |
| @@ -81,8 +81,8 @@ pub fn generate_error_details(metadata: &RuntimeMetadataV14) -> ErrorDetails { | |||
| }, | |||
| dispatch_error_impl_fn: quote! { | |||
| pub fn details(&self) -> Option<ErrorDetails> { | |||
| if let Self::Module { index, error } = self { | |||
| match (index, error) { | |||
| if let Self::Module (module_error) = self { | |||
There was a problem hiding this comment.
One downside of this is that this codegen is now not backwards compatible with older versions of substrate. Not sure what a nice solution to this is. Of course we can always detect whether it is the tuple variant of the old struct variant, but then that is not a nice solution :)
There was a problem hiding this comment.
Mmm yeah; the only solution that comes to mind is doing a proper check for it in the metadata and conditionally outputting one or the other. Perhaps that's worth doing though so that this passes CI now (and continues to pass when the change finally lands)?
Side note: I'm wondering why the change hasn't made it to the binary that CI uses yet. Any ideas? I may have to look into that!
There was a problem hiding this comment.
the only solution that comes to mind is doing a proper check for it in the metadata and conditionally outputting one or the other.
I agree, unfortunately that seems to be the only way to do this.
I'm wondering why the change hasn't made it to the binary that CI uses yet. Any ideas?
I thought it automatically uses a recent build from master? If that was working right it should be there. Unless something is being cached somewhere.
There was a problem hiding this comment.
Asked in the CI/CD room. We rely on a binary made available at https://releases.parity.io/substrate/x86_64-debian:stretch/latest/substrate/substrate from a nightly job on substrate gitlab, but the binary is from a commit ~21 days old..
There was a problem hiding this comment.
The nightly job could certainly build and run the latest binary from master, but doing so would slow down the regular CI a bunch I suspect, so it would keep failing (not sure how much caching would end up helping though), so I avoided it.
|
@jsdw Do you want to do the conditional check on the metadata to stay backwards compatible in this PR? Otherwise I'd be inclined to merge this. |
Lemme have a stab at backward compat :) |
Update the codegen to accomodate this change.
When nightly CI fails owing to a new substrate binary, this is hopefully the fix.