-
Notifications
You must be signed in to change notification settings - Fork 766
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
Migrating salary pallet to use umbrella crate #7048
Changes from 15 commits
1bf72c4
81ef8c3
12b5035
9b5c229
6a6a009
b015e26
53675b3
3291d1e
3dc4d8b
bf3302b
5397c4c
8db080b
c0b15a1
b8c1985
52a019b
f1e1408
023c3fe
6e83954
eeb1fc5
e539109
efeb7e6
3f30ef6
4595acf
49fb576
63b251c
8da372e
a2cf833
dcf3995
5900660
b427d2a
557bc71
de1b02f
4399f35
ae9b6e2
22ff507
9302fdc
e472261
9116f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 | ||
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json | ||
|
||
title: '[pallet-salary] Migrate to using frame umbrella crate' | ||
|
||
doc: | ||
- audience: Runtime Dev | ||
description: | ||
This PR migrates the pallet-salary to use the frame umbrella crate. | ||
This is part of the ongoing effort to migrate all pallets to use the frame umbrella crate. | ||
The effort is tracked [here](https://github.com/paritytech/polkadot-sdk/issues/6504). | ||
|
||
|
||
crates: | ||
- name: pallet-salary | ||
bump: minor | ||
- name: polkadot-sdk-frame | ||
bump: minor | ||
- name: traits-misc | ||
bump: minor | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,43 +17,22 @@ targets = ["x86_64-unknown-linux-gnu"] | |
|
||
[dependencies] | ||
codec = { features = ["derive"], workspace = true } | ||
frame-benchmarking = { optional = true, workspace = true } | ||
frame-support = { workspace = true } | ||
frame-system = { workspace = true } | ||
log = { workspace = true } | ||
pallet-ranked-collective = { optional = true, workspace = true } | ||
scale-info = { features = ["derive"], workspace = true } | ||
sp-arithmetic = { workspace = true } | ||
sp-core = { workspace = true } | ||
sp-io = { workspace = true } | ||
sp-runtime = { workspace = true } | ||
frame = { workspace = true, features = ["experimental", "runtime"] } | ||
|
||
[features] | ||
default = ["std"] | ||
std = [ | ||
"codec/std", | ||
"frame-benchmarking?/std", | ||
"frame-support/experimental", | ||
"frame-support/std", | ||
"frame-system/std", | ||
"log/std", | ||
"pallet-ranked-collective/std", | ||
"scale-info/std", | ||
"sp-arithmetic/std", | ||
"sp-core/std", | ||
"sp-io/std", | ||
"sp-runtime/std", | ||
"codec/std", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The taplo workflow is failing indicating that the formatting is indeed not needed / incorrect here. You need to check all failed workflows before merging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. taplo workflow is failing yes, but not for this file. let me revert it anyway, to a version before the taplo format |
||
"log/std", | ||
"pallet-ranked-collective/std", | ||
"scale-info/std", | ||
"frame/std", | ||
] | ||
runtime-benchmarks = [ | ||
"frame-benchmarking/runtime-benchmarks", | ||
"frame-support/runtime-benchmarks", | ||
"frame-system/runtime-benchmarks", | ||
"pallet-ranked-collective/runtime-benchmarks", | ||
"sp-runtime/runtime-benchmarks", | ||
] | ||
try-runtime = [ | ||
"frame-support/try-runtime", | ||
"frame-system/try-runtime", | ||
"pallet-ranked-collective?/try-runtime", | ||
"sp-runtime/try-runtime", | ||
"pallet-ranked-collective/runtime-benchmarks", | ||
"frame/runtime-benchmarks", | ||
] | ||
try-runtime = ["pallet-ranked-collective?/try-runtime", "frame/try-runtime"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,16 +19,10 @@ | |
|
||
#![cfg_attr(not(feature = "std"), no_std)] | ||
|
||
use codec::{Decode, Encode, MaxEncodedLen}; | ||
use core::marker::PhantomData; | ||
use scale_info::TypeInfo; | ||
use sp_arithmetic::traits::{Saturating, Zero}; | ||
use sp_runtime::{Perbill, RuntimeDebug}; | ||
|
||
use frame_support::{ | ||
defensive, | ||
dispatch::DispatchResultWithPostInfo, | ||
ensure, | ||
use frame::{ | ||
arithmetic::Perbill, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Arithmetic is already in prelude. - arithmetic::Perbill, |
||
prelude::*, | ||
traits::{ | ||
tokens::{GetSalary, Pay, PaymentStatus}, | ||
RankedMembers, RankedMembersSwapHandler, | ||
|
@@ -85,12 +79,9 @@ pub struct ClaimantStatus<CycleIndex, Balance, Id> { | |
status: ClaimState<Balance, Id>, | ||
} | ||
|
||
#[frame_support::pallet] | ||
#[frame::pallet] | ||
pub mod pallet { | ||
use super::*; | ||
use frame_support::{dispatch::Pays, pallet_prelude::*}; | ||
use frame_system::pallet_prelude::*; | ||
|
||
#[pallet::pallet] | ||
pub struct Pallet<T, I = ()>(PhantomData<(T, I)>); | ||
|
||
|
@@ -460,15 +451,15 @@ impl<T: Config<I>, I: 'static> | |
) { | ||
if who == new_who { | ||
defensive!("Should not try to swap with self"); | ||
return | ||
return; | ||
} | ||
if Claimant::<T, I>::contains_key(new_who) { | ||
defensive!("Should not try to overwrite existing claimant"); | ||
return | ||
return; | ||
} | ||
|
||
let Some(claimant) = Claimant::<T, I>::take(who) else { | ||
frame_support::defensive!("Claimant should exist when swapping"); | ||
defensive!("Claimant should exist when swapping"); | ||
return; | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,23 +17,15 @@ | |
|
||
//! The crate's tests. | ||
|
||
use std::collections::BTreeMap; | ||
|
||
use core::cell::RefCell; | ||
use frame_support::{ | ||
assert_noop, assert_ok, derive_impl, | ||
pallet_prelude::Weight, | ||
parameter_types, | ||
traits::{tokens::ConvertRank, ConstU64}, | ||
}; | ||
use sp_runtime::{traits::Identity, BuildStorage, DispatchResult}; | ||
|
||
use crate as pallet_salary; | ||
use crate::*; | ||
use core::cell::RefCell; | ||
use frame::{testing_prelude::*, traits::tokens::ConvertRank}; | ||
use std::collections::BTreeMap; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removing the line space between two import blocks causes them to be sorted alphabetically by the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I would prefer to not put in a separate block. If it is meant to be in a particular order then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. other comments addressed, leaving this as is |
||
|
||
type Block = frame_system::mocking::MockBlock<Test>; | ||
type Block = MockBlock<Test>; | ||
|
||
frame_support::construct_runtime!( | ||
construct_runtime!( | ||
pub enum Test | ||
{ | ||
System: frame_system, | ||
|
@@ -124,7 +116,7 @@ impl RankedMembers for TestClub { | |
} | ||
fn demote(who: &Self::AccountId) -> DispatchResult { | ||
CLUB.with(|club| match club.borrow().get(who) { | ||
None => Err(sp_runtime::DispatchError::Unavailable), | ||
None => Err(DispatchError::Unavailable), | ||
Some(&0) => { | ||
club.borrow_mut().remove(&who); | ||
Ok(()) | ||
|
@@ -156,9 +148,9 @@ impl Config for Test { | |
type Budget = Budget; | ||
} | ||
|
||
pub fn new_test_ext() -> sp_io::TestExternalities { | ||
pub fn new_test_ext() -> TestState { | ||
let t = frame_system::GenesisConfig::<Test>::default().build_storage().unwrap(); | ||
let mut ext = sp_io::TestExternalities::new(t); | ||
let mut ext = TestState::new(t); | ||
ext.execute_with(|| System::set_block_number(1)); | ||
ext | ||
} | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,15 +66,15 @@ impl<T: VariantCount> Get<u32> for VariantCountOf<T> { | |
#[macro_export] | ||
macro_rules! defensive { | ||
() => { | ||
frame_support::__private::log::error!( | ||
$crate::__private::log::error!( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. very good! |
||
target: "runtime::defensive", | ||
"{}", | ||
$crate::traits::DEFENSIVE_OP_PUBLIC_ERROR | ||
); | ||
debug_assert!(false, "{}", $crate::traits::DEFENSIVE_OP_INTERNAL_ERROR); | ||
}; | ||
($error:expr $(,)?) => { | ||
frame_support::__private::log::error!( | ||
$crate::__private::log::error!( | ||
target: "runtime::defensive", | ||
"{}: {:?}", | ||
$crate::traits::DEFENSIVE_OP_PUBLIC_ERROR, | ||
|
@@ -83,7 +83,7 @@ macro_rules! defensive { | |
debug_assert!(false, "{}: {:?}", $crate::traits::DEFENSIVE_OP_INTERNAL_ERROR, $error); | ||
}; | ||
($error:expr, $proof:expr $(,)?) => { | ||
frame_support::__private::log::error!( | ||
$crate::__private::log::error!( | ||
target: "runtime::defensive", | ||
"{}: {:?}: {:?}", | ||
$crate::traits::DEFENSIVE_OP_PUBLIC_ERROR, | ||
|
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.
You shouldn't change the indentation here, it also makes it harder to review. Probably it's a matter of the toml formatter you set up
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.
yup this is from taplo format