-
Notifications
You must be signed in to change notification settings - Fork 804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate pallet-aura
to umbrella crate
#6622
base: master
Are you sure you want to change the base?
Conversation
substrate/frame/aura/src/lib.rs
Outdated
BoundedSlice, BoundedVec, ConsensusEngineId, Parameter, | ||
use frame::{ | ||
deps::{ | ||
frame_support::{ConsensusEngineId, Parameter}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please read the new checklist named in the issue, and update the PR to adhere to it. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix failing CI jobs (missing imports)
please review @kianenigma @re-gius |
substrate/frame/src/lib.rs
Outdated
@@ -363,6 +366,9 @@ pub mod runtime { | |||
/// Consider using the new version of this [`frame_construct_runtime`]. | |||
pub use frame_support::construct_runtime; | |||
|
|||
/// Related to runtime construction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is not relevant for me
substrate/frame/aura/src/lib.rs
Outdated
BoundedSlice, BoundedVec, ConsensusEngineId, Parameter, | ||
use frame::{ | ||
runtime::prelude::*, | ||
traits::{DisabledValidators, FindAuthor, IsMember, OnTimestampSet, OneSessionHandler}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add IsMember
to the main prelude of the umbrella crate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also moved OnTimestampSet
to top level prelude, used in Babe and Aura pallets.
FindAuthor
& DisableValidators
to account
mod
substrate/frame/aura/src/lib.rs
Outdated
traits::{DisabledValidators, FindAuthor, Get, OnTimestampSet, OneSessionHandler}, | ||
BoundedSlice, BoundedVec, ConsensusEngineId, Parameter, | ||
use frame::{ | ||
runtime::prelude::*, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the main prelude frame::prelude::*
instead of the runtime one
substrate/frame/src/lib.rs
Outdated
@@ -363,6 +366,9 @@ pub mod runtime { | |||
/// Consider using the new version of this [`frame_construct_runtime`]. | |||
pub use frame_support::construct_runtime; | |||
|
|||
/// Related to runtime construction. | |||
pub use frame_support::{BoundedSlice, BoundedVec}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you added them just to use the runtime prelude in the aura
pallet, I think it's better to change it to the main prelude (see comment) and remove this from here
please pause work on migrating polkadot-sdk pallets to umbrella crate until we come to a final decision here: #6504 (comment) |
Part of #6504
Description
sp-application-crypto
in polkadot-sdk-frame crate, re-exportsRuntimeAppPublic
andBoundToRuntimeAppPublic
as part of the cryptography module.