Skip to content
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

pallet ranked collective: max member count per rank #4807

Merged
merged 5 commits into from
Jun 24, 2024

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Jun 17, 2024

Configuration for the maximum member count per rank, with the option for no limit.

@muharem muharem added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Jun 17, 2024
@muharem muharem requested a review from a team as a code owner June 17, 2024 10:07
@muharem muharem requested review from ggwpez and joepetrowski June 17, 2024 10:13
/// Therefore, the limit `m` at rank `x` sets the maximum total member count for rank `x`
/// and all ranks above.
/// The `None` indicates no member count limit for the given rank.
type MaxMemberCount: MaybeConvert<Rank, MemberIndex>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a default config for this pallet? Would be nice to set this to () to avoid the breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it does not have. There is also a discussion to use default configs only for tests and maybe common configs for system parachains https://forum.polkadot.network/t/seeking-feedback-on-derive-impl-for-default-configurations-in-pallet-templates/7584

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also here #4689

Copy link
Member

Choose a reason for hiding this comment

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

I think there should still be a trivial implementation of this to reduce copy&paste.
Some struct like UnlimitedMembersPerRank or so that just always returns 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.

MaybeConvert has a tuple implementation, which always returns none

Copy link
Member

Choose a reason for hiding this comment

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

Oh, so it is possible to just pass ()? Then i misunderstood the discussion, good!

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

prdoc/pr_4807.prdoc Outdated Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team June 17, 2024 12:18
@muharem
Copy link
Contributor Author

muharem commented Jun 24, 2024

bot merge

@command-bot
Copy link

command-bot bot commented Jun 24, 2024

@muharem bot merge and bot rebase are not supported anymore. Please use native Github "Auto-Merge" and "Update Branch" buttons instead.
image

@muharem muharem enabled auto-merge June 24, 2024 10:57
@muharem muharem added this pull request to the merge queue Jun 24, 2024
Merged via the queue into master with commit 0b11c27 Jun 24, 2024
153 of 158 checks passed
@muharem muharem deleted the muharem-ranked-collective-max-member-count branch June 24, 2024 12:03
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
Configuration for the maximum member count per rank, with the option for
no limit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants