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

Try State Hook for Ranked Collective #3007

Merged

Conversation

dharjeezy
Copy link
Contributor

@dharjeezy dharjeezy commented Jan 21, 2024

Part of: #239

Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW

@dharjeezy dharjeezy marked this pull request as ready for review January 24, 2024 21:55
@dharjeezy dharjeezy requested review from a team January 24, 2024 21:55
Copy link
Contributor

@liamaharon liamaharon left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

Great start, but has a few more invariants to go.

substrate/frame/ranked-collective/src/lib.rs Outdated Show resolved Hide resolved
substrate/frame/ranked-collective/src/lib.rs Show resolved Hide resolved
substrate/frame/ranked-collective/src/lib.rs Show resolved Hide resolved
@dharjeezy dharjeezy requested a review from a team as a code owner February 5, 2024 10:25
…to dami/try-state-ranked-collective

� Conflicts:
�	substrate/frame/ranked-collective/src/lib.rs
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

I cannot be sure that all invariants are correctly captured here, but otherwise it is a good textbook example of hwat needs to be done.

Can you check a possible invariant as with #3156 (comment)? If there are a list of members being shared.

Lastly, can you provide your DOT/KSM Address for a tip?

@dharjeezy
Copy link
Contributor Author

I cannot be sure that all invariants are correctly captured here, but otherwise it is a good textbook example of hwat needs to be done.

Can you check a possible invariant as with #3156 (comment)? If there are a list of members being shared.

Lastly, can you provide your DOT/KSM Address for a tip?

I have provided my Polkadot address in the PR description @kianenigma

@kianenigma
Copy link
Contributor

/tip medium

Copy link

@kianenigma A referendum for a medium (80 DOT) tip was successfully submitted for @dharjeezy (12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW on polkadot).

https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Frpc.polkadot.io#/referenda tip

Copy link
Contributor

@liamaharon liamaharon left a comment

Choose a reason for hiding this comment

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

This is close, but not capturing all invariants I would consider important to check. Remember, we want to ensure all the state across all storage items is consistent. If you have any questions about my comments feel free to ask.

substrate/frame/ranked-collective/src/lib.rs Show resolved Hide resolved
substrate/frame/ranked-collective/src/lib.rs Outdated Show resolved Hide resolved
substrate/frame/ranked-collective/src/lib.rs Outdated Show resolved Hide resolved
substrate/frame/ranked-collective/src/lib.rs Show resolved Hide resolved
@liamaharon
Copy link
Contributor

@dharjeezy can you take a look at the failing ci?

@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/5153951

@liamaharon liamaharon added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Feb 8, 2024
@liamaharon liamaharon added this pull request to the merge queue Feb 8, 2024
Merged via the queue into paritytech:master with commit 9cd02a0 Feb 8, 2024
120 of 125 checks passed
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
Part of: paritytech#239

Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW

---------

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
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