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

Exposed add_member_to_rank #12050

Closed
wants to merge 5 commits into from
Closed

Conversation

MrishoLukamba
Copy link
Contributor

Exposed the extrinsic call only root origin can call.
Performed testing..
Before continuing I need clarifications If i got the instructions right @shawntabrizi
Remaining the first task also to include it as kitchen sink

@cla-bot-2021
Copy link

cla-bot-2021 bot commented Aug 16, 2022

User @MrishoLukamba, please sign the CLA here.

@MrishoLukamba
Copy link
Contributor Author

This PR is on issue paritytech/polkadot-sdk#262

member: T::AccountId,
rank: Rank
) -> DispatchResult {
let _= ensure_root(origin)?;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let _= ensure_root(origin)?;
ensure_root(origin)?;

Comment on lines 482 to 483
Self::do_add_member_to_rank(member,rank)?;
Ok(())
Copy link
Member

@shawntabrizi shawntabrizi Aug 16, 2022

Choose a reason for hiding this comment

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

Suggested change
Self::do_add_member_to_rank(member,rank)?;
Ok(())
Self::do_add_member_to_rank(member, rank)

@@ -471,6 +471,18 @@ pub mod pallet {
Self::do_add_member(who)
}

// Only root account can add a member to
#[pallet::weight(100)]
Copy link
Member

Choose a reason for hiding this comment

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

need proper weights and benchmark

Comment on lines 52 to 56
impl pallet_sudo::Config for Test {
type Event = Event;
type Call = Call;
}

Copy link
Member

Choose a reason for hiding this comment

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

you dont need this, so please remove all the stuff related to sudo

@shawntabrizi
Copy link
Member

shawntabrizi commented Aug 16, 2022

You should always add the new extrinsic at the bottom of the extrinsics to keep the existing order of the previous extrinsics.

@ggwpez ggwpez added the A3-in_progress Pull request is in progress. No review needed at this stage. label Aug 16, 2022
@MrishoLukamba
Copy link
Contributor Author

okey am on it, thanks

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1756211

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1756210

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
The job name - cargo-check-benches
The job logs - https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/1756292

@shawntabrizi
Copy link
Member

@MrishoLukamba please keep this PR minimal and only adding the new extrinsic. you can open a new PR for the other fellowship feature

@MrishoLukamba
Copy link
Contributor Author

This is outdated

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A3-in_progress Pull request is in progress. No review needed at this stage.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants