Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Deprecate Pallet decl_* Macros #13705

Merged
merged 10 commits into from
May 9, 2023
Merged

Deprecate Pallet decl_* Macros #13705

merged 10 commits into from
May 9, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Mar 24, 2023 •
edited
Loading

It should come at no surprise that we are deprecating: decl_module, decl_storage, decl_error and decl_event.
Any remaining usage should be migrated to use #[frame_support::pallet].

Related #12248
Depends on #12445, #12401

ggwpez added 2 commits March 24, 2023 16:47
@ggwpez
Deprecate decl_ macros
1cb3433 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Allow deprecated in tests
ce36080 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. labels Mar 24, 2023
@bkchr
Copy link
Member

bkchr commented Mar 24, 2023

Don't we first want to merge the prs that remove the usages in our code base?

@altaua
Copy link
Contributor

altaua commented Mar 24, 2023

here's the actual reason for the pipeline failure, the decl_storage_ui test is unsurpringly failing. Will investigate how the job log got truncated.

@ggwpez
Copy link
Member Author

ggwpez commented Mar 24, 2023

Don't we first want to merge the prs that remove the usages in our code base?

Ideally yes, but they got closed as stale. Will need to check how much effort that is and maybe quickly do it.
Otherwise this is already going to notify any other downstream projects who are still using them.

@bkchr
Copy link
Member

bkchr commented Mar 24, 2023

They got reopened. Otherwise it should be fairly straightforward to remove them.

@ggwpez ggwpez added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Mar 25, 2023
@stale
Copy link

stale bot commented Apr 24, 2023

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A3-stale label Apr 24, 2023
@ggwpez ggwpez removed the A3-stale label Apr 25, 2023
@ggwpez
Copy link
Member Author

ggwpez commented Apr 26, 2023

Okay will wait for the other MRs.

@ggwpez ggwpez closed this Apr 26, 2023
@ggwpez ggwpez reopened this May 9, 2023
@ggwpez ggwpez requested a review from a team May 9, 2023 09:33
@ggwpez
Deprecate all
de3dcc1 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented May 9, 2023 •
edited
Loading

I searched on GitHub, and there are still lots of projects using the old macros. I think we should give at least 3 months lead period before finally deleting. Although, this old code is blocking a lot of issues…
cc @juangirini @aaronbassett

@ggwpez
Push missing files
92c3a40 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez changed the title Officially deprecate pallet decl_* macros Deprecate Pallet decl_* Macros May 9, 2023
@ggwpez
fmt
b5f15a3 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added C3-medium PR touches the given topic and has a medium impact on builders. and removed C1-low PR touches the given topic and has a low impact on builders. A3-in_progress Pull request is in progress. No review needed at this stage. labels May 9, 2023
@ggwpez ggwpez added the A0-please_review Pull request needs code review. label May 9, 2023
sam0x17
sam0x17 approved these changes May 9, 2023
View reviewed changes
Copy link
Contributor

@sam0x17 sam0x17 left a comment

Choose a reason for hiding this comment

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

Yeah I agree that deprecation and then eventual removal is definitely the way to go

@ggwpez ggwpez added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels May 9, 2023
ggwpez added 3 commits May 9, 2023 12:40
@ggwpez
Move decl_module to own file
543a311 
Otherwise i have to spam allow(deprecated) for all recursive calls.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Revert "Move decl_module to own file"
7f2ded8 
This reverts commit 543a311.
@ggwpez
Fix tests
b4e8619 
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
ggwpez
ggwpez commented May 9, 2023
View reviewed changes
frame/support/src/dispatch.rs
@@ -3197,6 +3199,7 @@ macro_rules! __check_reserved_fn_name {
#[cfg(test)]
// Do not complain about unused `dispatch` and `dispatch_aux`.
#[allow(dead_code)]
#[allow(deprecated)]
Copy link
Member Author

Choose a reason for hiding this comment

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

Test will be deleted once the syntax is gone.

@ggwpez
Copy link
Member Author

ggwpez commented May 9, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 2a148e3 into master May 9, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-deprecate-decl-macros branch May 9, 2023 14:28
@github-actions github-actions bot mentioned this pull request Jun 15, 2023
Closed
@kiltbot kiltbot mentioned this pull request Jun 21, 2023
Closed
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@ggwpez @nathanwhit
Deprecate Pallet decl_* Macros (paritytech#13705)
e284da2 
* Deprecate decl_ macros

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Allow deprecated in tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Revert "Allow deprecated in tests"

This reverts commit ce36080.

* Deprecate all

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Push missing files

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* fmt

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Move decl_module to own file

Otherwise i have to spam allow(deprecated) for all recursive calls.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Revert "Move decl_module to own file"

This reverts commit 543a311.

* Fix tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@surangap surangap mentioned this pull request May 29, 2024
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KiChjang KiChjang KiChjang approved these changes

@sam0x17 sam0x17 sam0x17 approved these changes

Assignees
No one assigned
Labels
A3-in_progress Pull request is in progress. No review needed at this stage. B1-note_worthy Changes should be noted in the release notes C3-medium PR touches the given topic and has a medium impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Runtime / FRAME
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ggwpez @bkchr @altaua @sam0x17 @KiChjang