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

Migrate pallet-paged-list to umbrella crate #6931

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

Krayt78
Copy link
Contributor

@Krayt78 Krayt78 commented Dec 17, 2024

Part of #6504

@Krayt78
Copy link
Contributor Author

Krayt78 commented Dec 17, 2024

had to add frame support for defensive trait
for some reason i have a unused warnin but it is used

@Krayt78 Krayt78 marked this pull request as ready for review December 18, 2024 08:32
@Krayt78 Krayt78 requested a review from a team as a code owner December 18, 2024 08:32
@kianenigma kianenigma added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Dec 19, 2024
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.

Good first steps, thank you!

Please see my comments + you have compilation errors/warnings, so the CI is not passing yet.

@Krayt78
Copy link
Contributor Author

Krayt78 commented Dec 19, 2024

@ggwpez Hi Oliver ! Would you know if this line can be safely removed ? idk if it has any side effects on tests but it doesnt seem to do at first glance https://github.com/paritytech/polkadot-sdk/blame/master/substrate/frame/paged-list/src/paged_list.rs#L448

@ggwpez
Copy link
Member

ggwpez commented Jan 2, 2025

Would you know if this line can be safely removed ? idk if it has any side effects on tests but it doesnt seem to do at first glance

Cannot be removed, see its drop.
It is equivalent to wrapping everything below into an assert_storage_noop! to ensure that there are no undue storage changes happening.

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Good minus the one comment 😄

@paritytech-review-bot paritytech-review-bot bot requested a review from a team January 4, 2025 19:48
@Krayt78 Krayt78 requested review from kianenigma and ggwpez January 5, 2025 09:09
@Krayt78
Copy link
Contributor Author

Krayt78 commented Jan 7, 2025

I added all traits that were not used by multiple pallets to the umbrella crate and integrated the defensive! macro changes following the merge of #7069

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.

3 participants