Retain specific runtime APIs #961
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsdw
reviewed
May 19, 2023
cli/src/commands/metadata.rs
Outdated
Comment on lines
52
to
70
| match (opts.pallets.as_ref(), opts.runtime_apis.as_ref()) { | ||
| (Some(pallets), Some(runtime_apis)) => retain_metadata( | ||
| &mut metadata_v15, | ||
| |pallet_name| pallets.iter().any(|p| &**p == pallet_name), | ||
| |runtime_api_name| runtime_apis.iter().any(|p| &**p == runtime_api_name), | ||
| ), | ||
| (Some(pallets), None) => retain_metadata( | ||
| &mut metadata_v15, | ||
| |pallet_name| pallets.iter().any(|p| &**p == pallet_name), | ||
| |_| true, | ||
| ), | ||
| (None, Some(runtime_apis)) => retain_metadata( | ||
| &mut metadata_v15, | ||
| |_| true, | ||
| |runtime_api_name| runtime_apis.iter().any(|p| &**p == runtime_api_name), | ||
| ), | ||
| (None, None) => {} | ||
| } | ||
| metadata = metadata_v15.into(); |
There was a problem hiding this comment.
The way to avoid the repetition would be by boxing the closures like so:
let retain_pallets_fn: Box<dyn Fn(&str) -> bool> = match opts.pallets.as_ref() {
Some(pallets) => Box::new(|name| pallets.iter().any(|p| &**p == name)),
None => Box::new(|_| true)
};
let retain_runtime_apis_fn: Box<dyn Fn(&str) -> bool> = match opts.runtime_apis.as_ref() {
Some(apis) => Box::new(|name| apis.iter().any(|p| &**p == name)),
None => Box::new(|_| true)
};
retain_metadata(&mut metadata_v15, retain_pallets_fn, retain_runtime_apis_fn);
metadata = metadata_v15.into();A little extra overhead but not a big deal :)
There was a problem hiding this comment.
Ah, nice, yeah that is definitely a solution. I still want to keep a check that at least one of the arguments is set, because if not we can avoid calling the retain_metadata function alltogether.
There was a problem hiding this comment.
Yeah I wouldn't reaplce the initial check, just the highlighted lines :)
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #933
I integrated the retaining of specific runtime APIs into the existing
retain_metadata_palletsfunctionality, because it is more efficient to join the two operations in one place. As for the typing: I tried usindOption<FnMut(&str) -> bool>as a sensiple argument type for bothpallets_filterandruntime_apis_filterbut spent way too much time on lifetime issues and could not get it going well. So now I settled for just passing|_| trueclosures, when all elements should be retained. Not so nice, I know, but should work.