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

membership: Restructure pallet into separate files #4536

Merged
merged 9 commits into from
Jul 24, 2024

Conversation

vatsa287
Copy link
Contributor

@vatsa287 vatsa287 commented May 22, 2024

  • What does this PR do?
    This PR separates membership pallet into separate files for lib, mock & tests.

  • Why are these changes needed?
    Currently membership pallet consists of lib, mock & tests written into a single file.
    Refactor it into separate files which makes it inline to other pallets and improves readability.

  • How were these changes implemented and what do they affect?
    The PR will not have any affect.

Signed-off-by: Shreevatsa N i.mnshreevatsa@gmail.com

@vatsa287 vatsa287 requested a review from a team as a code owner May 22, 2024 05:27
@cla-bot-2021
Copy link

cla-bot-2021 bot commented May 22, 2024

User @vatsa287, please sign the CLA here.

@vatsa287
Copy link
Contributor Author

Label required: T1-Frame.
Not able to add this into the PR.

@xlc
Copy link
Contributor

xlc commented May 22, 2024

you forget to commit the lib.rs file?

@vatsa287
Copy link
Contributor Author

vatsa287 commented May 22, 2024

you forget to commit the lib.rs file?

@xlc Sorry yes. I have updated the commit now.
Thanks.

@ggwpez ggwpez self-requested a review May 24, 2024 20:31
@ggwpez ggwpez self-requested a review May 24, 2024 20:32
@ggwpez
Copy link
Member

ggwpez commented May 24, 2024

Please check that everything still works when running the tests.

Copy link

Review required! Latest push from author must always be reviewed

@vatsa287 vatsa287 force-pushed the refactor-membership branch 2 times, most recently from 5f37907 to 4ddacee Compare May 26, 2024 06:58
@vatsa287
Copy link
Contributor Author

Please check that everything still works when running the tests.

Yes it works fine on my local setup.

vatsa@Shreevatsas-MacBook-Pro polkadot-sdk % cargo test -p pallet-membership --features=runtime-benchmarks
    Finished `test` profile [unoptimized + debuginfo] target(s) in 1.60s
     Running unittests src/lib.rs (target/debug/deps/pallet_membership-f44be527ad6bc1b4)

running 22 tests
test mock::__construct_runtime_integrity_test::runtime_integrity_tests ... ok
test mock::test_genesis_config_builds ... ok
test benchmarking::bench_clear_prime ... ok
test benchmarking::bench_set_prime ... ok
test benchmarking::bench_swap_member ... ok
test tests::add_member_works ... ok
test benchmarking::bench_change_key ... ok
test benchmarking::bench_reset_members ... ok
test benchmarking::bench_remove_member ... ok
test benchmarking::bench_add_member ... ok
test tests::change_key_with_same_caller_as_argument_changes_nothing ... ok
test tests::change_key_works_that_does_not_change_order ... ok
test tests::change_key_works ... ok
test tests::query_membership_works ... ok
test tests::swap_member_with_identical_arguments_changes_nothing ... ok
test tests::prime_member_works ... ok
test tests::remove_member_works ... ok
test tests::reset_members_works ... ok
test tests::swap_member_works ... ok
test tests::swap_member_works_that_does_not_change_order ... ok
test tests::migration_v4 ... ok
test tests::genesis_build_panics_with_duplicate_members - should panic ... ok

test result: ok. 22 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

   Doc-tests pallet-membership

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

@@ -33,6 +33,15 @@ use sp_std::prelude::*;
pub mod migrations;
pub mod weights;

#[cfg(any(feature = "mock", test))]
Copy link
Member

Choose a reason for hiding this comment

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

Does this mock feature exist? I dont think its needed otherwise.

Copy link
Contributor Author

@vatsa287 vatsa287 May 27, 2024

Choose a reason for hiding this comment

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

Yes mock is the file where mock runtime is defined, required for test.

Signed-off-by: Shreevatsa N <i.mnshreevatsa@gmail.com>
Signed-off-by: Shreevatsa N <i.mnshreevatsa@gmail.com>
@vatsa287
Copy link
Contributor Author

@ggwpez any other changes required? Could you please approve the workflow.
Thanks.

substrate/frame/membership/src/lib.rs Outdated Show resolved Hide resolved
substrate/frame/membership/src/tests.rs Outdated Show resolved Hide resolved
substrate/frame/membership/src/tests.rs Outdated Show resolved Hide resolved
@bkchr bkchr enabled auto-merge June 25, 2024 11:57
@bkchr bkchr added the R0-silent Changes should not be mentioned in any release notes label Jun 25, 2024
@github-actions github-actions bot requested a review from bkchr June 25, 2024 16:43
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6795541

@bkchr bkchr added this pull request to the merge queue Jul 24, 2024
Merged via the queue into paritytech:master with commit f5e7eaf Jul 24, 2024
158 of 160 checks passed
@vatsa287
Copy link
Contributor Author

@ggwpez @bkchr
Thanks for the help!

TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
- What does this PR do?
This PR separates `membership` pallet into separate files for `lib`,
`mock` & `tests`.
   
- Why are these changes needed?
Currently `membership` pallet consists of `lib`, `mock` & `tests`
written into a single file.
Refactor it into separate files which makes it inline to other pallets
and improves readability.

- How were these changes implemented and what do they affect?
   The PR will not have any affect.

Signed-off-by: Shreevatsa N <i.mnshreevatsa@gmail.com>

---------

Signed-off-by: Shreevatsa N <i.mnshreevatsa@gmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <info@kchr.de>
ordian added a commit that referenced this pull request Aug 6, 2024
* master: (27 commits)
  Bridges improved tests and nits (#5128)
  Fix misleading comment about RewardHandler in epm config (#3095)
  Introduce a workflow updating the wishlist leaderboards (#5085)
  membership: Restructure pallet into separate files (#4536)
  Fix after ring-proof api change (#5126)
  Bump paritytech/review-bot from 2.4.0 to 2.5.0 (#5057)
  Bump docker/login-action from 3.0.0 to 3.3.0 (#5109)
  Bump docker/build-push-action from 5.1.0 to 6.5.0 (#5108)
  Bump peter-evans/create-pull-request from 5.0.0 to 6.1.0 (#5093)
  Tx Payment: drop ED requirements for tx payments with exchangeable asset  (#4488)
  Remove `pallet-getter` usage from pallet-transaction-payment (#4970)
  pallet macro: do not generate try-runtime related code when frame-support doesn't have try-runtime. (#5099)
  fix(chain-spec): ChainSpecBuilder with object as default genesis (#4345)
  Migrate BEEFY BLS crypto to  bls12-381 curve (#4931)
  Bump clap from 4.5.9 to 4.5.10 in the known_good_semver group (#5120)
  Use jobserver in wasm-builder to limit concurrency of spawned cargo processes (#4946)
  include events for voting (#4613)
  [subsystem-bench] Add mocks for own assignments triggering (#5042)
  Remove not-audited warning (#5114)
  hotfix: blockchain/backend: Skip genesis leaf to unblock syncing (#5103)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants