-
Notifications
You must be signed in to change notification settings - Fork 803
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-nomination-pools and pallet-nomination-pools-runtime-api to use umbrella crate #7218
base: master
Are you sure you want to change the base?
Conversation
Clean up files like |
"sp-tracing?/std", | ||
] | ||
runtime-benchmarks = [ | ||
"frame-support/runtime-benchmarks", | ||
"frame-system/runtime-benchmarks", | ||
"pallet-balances?/runtime-benchmarks", |
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.
there is no frame/runtime-benchmarks
?
DefaultNoBound, PalletError, | ||
}; | ||
use frame_system::pallet_prelude::BlockNumberFor; | ||
|
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.
FixedPointNumber, Perbill, | ||
}; | ||
use sp_staking::{EraIndex, StakingInterface}; | ||
|
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.
@@ -2667,7 +2648,7 @@ pub mod pallet { | |||
) -> DispatchResult { | |||
let mut bonded_pool = match ensure_root(origin.clone()) { | |||
Ok(()) => BondedPool::<T>::get(pool_id).ok_or(Error::<T>::PoolNotFound)?, | |||
Err(sp_runtime::traits::BadOrigin) => { | |||
Err(_bad_origin) => { |
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.
why not keep the exact match here?
Err(_bad_origin) => { | |
Err(BadOrigin) => { |
use frame_system::{EnsureSignedBy, RawOrigin}; | ||
use sp_runtime::{BuildStorage, DispatchResult, FixedU128}; | ||
use sp_staking::{ | ||
|
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.
FixedU128, | ||
}; | ||
use sp_staking::{Agent, DelegationInterface}; | ||
|
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.
pub use frame_support::{ | ||
defensive, defensive_assert, ensure, | ||
traits::{ | ||
fungible::{Inspect, InspectFreeze, Mutate, MutateFreeze}, |
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 don't think we should import the fungible trait like this. they will collide with fungibles
one.
}; | ||
|
||
pub use frame_support::traits::tokens::{Fortitude::Polite, Preservation::Expendable, Balance}; |
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.
IMO importing enum variant is too much, we should import only Fortitude and Preservation.
OneSessionHandler, RankedMembers, RankedMembersSwapHandler, StorageVersion, | ||
UncheckedOnRuntimeUpgrade, VariantCountOf, | ||
}, | ||
PalletError, PalletId, PartialEqNoBound |
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.
As a note, I think PartialEqNoBound is already imported in derive.
@@ -326,9 +339,11 @@ pub mod testing_prelude { | |||
/// Other helper macros from `frame_support` that help with asserting in tests. | |||
pub use frame_support::{ | |||
assert_err, assert_err_ignore_postinfo, assert_error_encoded_size, assert_noop, assert_ok, | |||
assert_storage_noop, ensure, hypothetically, storage_alias, | |||
assert_storage_noop, hypothetically, storage_alias, DefaultNoBound, |
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.
why is ensure
removed? DefaultNoBound
should already be in prelude through derive IIRC
Part of #6504
Polkadot Address: 121HJWZtD13GJQPD82oEj3gSeHqsRYm1mFgRALu4L96kfPD1