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.

Sign up for GitHub

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

Jump to bottom

Try State Hook for Ranked Collective #3007

Merged
merged 21 commits into from
Feb 8, 2024
Merged

Try State Hook for Ranked Collective #3007

merged 21 commits into from
Feb 8, 2024

Conversation

dharjeezy
Copy link
Contributor

@dharjeezy dharjeezy commented Jan 21, 2024 •
edited
Loading

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
@liamaharon liamaharon mentioned this pull request Jan 26, 2024
Open
62 tasks
liamaharon
liamaharon reviewed Jan 26, 2024
View reviewed changes
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
@dharjeezy
Merge branch 'master' of https://github.com/dharjeezy/polkadot-sdk in…
76edeaf 
…to dami/try-state-ranked-collective

� Conflicts:
�	substrate/frame/ranked-collective/src/lib.rs
@dharjeezy
fix test
71ae86f
@dharjeezy dharjeezy requested a review from liamaharon February 5, 2024 10:52
kianenigma
kianenigma approved these changes Feb 5, 2024
View reviewed changes
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

@substrate-tip-bot substrate-tip-bot
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

liamaharon
liamaharon suggested changes Feb 6, 2024
View reviewed changes
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
@dharjeezy dharjeezy requested a review from liamaharon February 6, 2024 10:12
@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 enabled auto-merge February 8, 2024 13:52
@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
@dharjeezy @liamaharon
Try State Hook for Ranked Collective (paritytech#3007)
5b22c03 
Part of: paritytech#239

Polkadot address: 12GyGD3QhT4i2JJpNzvMf96sxxBLWymz4RdGCxRH5Rj5agKW

---------

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
This was referenced Jun 5, 2024
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liamaharon liamaharon liamaharon approved these changes

@kianenigma kianenigma kianenigma approved these changes

Assignees
No one assigned
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dharjeezy @kianenigma @liamaharon @paritytech-cicd-pr