Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Replace last usages of <() as PalletInfo> in substrate #8080

Merged
5 commits merged into from
Feb 9, 2021

Conversation

gui1117
Copy link
Contributor

@gui1117 gui1117 commented Feb 9, 2021

related #7949

@gui1117 gui1117 added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Feb 9, 2021

None
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I didn't use construct_runtime here because runtime declare a very specific implementation of Call which is not compatible with the code generated by construct_runtime


None
}
}
Copy link
Contributor Author

@gui1117 gui1117 Feb 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I didn't use construct_runtime here because I don't think it is really needed for the test, and the easiest is a manual implementation.

gui1117 and others added 2 commits February 9, 2021 12:19
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@gui1117 gui1117 requested a review from ascjones February 9, 2021 11:21
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides the duplicate, it looks good

Comment on lines 46 to 55
pub struct PanicPalletInfo;

impl frame_support::traits::PalletInfo for PanicPalletInfo{
fn index<P: 'static>() -> Option<usize> {
unimplemented!("PanicPalletInfo mustn't be triggered by tests");
}
fn name<P: 'static>() -> Option<&'static str> {
unimplemented!("PanicPalletInfo mustn't be triggered by tests");
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you use it from frame-support?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in frame-support I didn't expose it, it is hidden behind the #[cfg(test)] flag, but we could make it public.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh I had overseen this. Maybe move it at least from the traits file then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I moved it to tests module

@bkchr
Copy link
Member

bkchr commented Feb 9, 2021

bot merge

@ghost
Copy link

ghost commented Feb 9, 2021

Waiting for commit status.

@ghost ghost merged commit 273bc7b into master Feb 9, 2021
@ghost ghost deleted the gui-finish-replacing-pallet-info branch February 9, 2021 15:28
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants