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

Migrate remaining old decl_* macros to the new pallet attribute macros#12271

Merged
paritytech-processbot[bot] merged 11 commits intoparitytech:masterfrom
koushiro:remove-old-decl-macros
Sep 22, 2022
Merged

Migrate remaining old decl_* macros to the new pallet attribute macros#12271
paritytech-processbot[bot] merged 11 commits intoparitytech:masterfrom
koushiro:remove-old-decl-macros

Conversation

@koushiro
Copy link
Copy Markdown
Contributor

@koushiro koushiro commented Sep 15, 2022

What does it do?

Migrate remaining old decl_* macros to the new #[pallet] attribute macros

part of #12248

polkadot address: 15XuanNimo5951s3RjFTPX1AvYVnCwfr3SDhb3AV4fQF3LpK

@koushiro
Migrate remaining old decl_* macros to the new pallet attribute macros
7e0ed4c 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
bkchr
bkchr reviewed Sep 15, 2022
View reviewed changes
Comment thread frame/benchmarking/src/tests.rs
#[pallet::pallet]
#[pallet::generate_store(pub(super) trait Store)]
pub struct Pallet<T>(_);
pub struct Pallet<T>(PhantomData<T>);
Copy link
Copy Markdown
Member

@bkchr bkchr Sep 15, 2022

Choose a reason for hiding this comment

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

Suggested change
pub struct Pallet<T>(PhantomData<T>);
pub struct Pallet<T>(_);

Copy link
Copy Markdown
Contributor Author

@koushiro koushiro Sep 15, 2022

Choose a reason for hiding this comment

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

Why not use struct Pallet<T>(PhantomData<T>);?

struct Pallet<T>(_); is not standard rust syntax

Copy link
Copy Markdown
Member

@bkchr bkchr Sep 15, 2022

Choose a reason for hiding this comment

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

Yeah I know. This syntax was added as some shortcut.

Copy link
Copy Markdown
Member

@shawntabrizi shawntabrizi Sep 15, 2022

Choose a reason for hiding this comment

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

i would not be against removing these shortcuts if you find it confusing. End of the day, no one writes this line by hand, they just copy some template.

Comment thread test-utils/runtime/src/system.rs Outdated
Comment thread utils/frame/rpc/support/src/lib.rs Outdated
@koushiro
Apply review suggestions
54afbca 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Apply review suggestions
99d9f6a 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro koushiro requested a review from bkchr September 16, 2022 03:17
bkchr
bkchr reviewed Sep 16, 2022
View reviewed changes
Comment thread test-utils/runtime/src/system.rs Outdated
bkchr
bkchr approved these changes Sep 17, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

CI isn't happy, but looks good otherwise!

Ty

@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Sep 20, 2022

@KiChjang @shawntabrizi PTAL

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Sep 20, 2022

@koushiro tests are still failing.

@koushiro
Fix dev rpc test
78521d7 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Fix service tests
fe4f7c5 
Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@koushiro
Copy link
Copy Markdown
Contributor Author

koushiro commented Sep 20, 2022

@koushiro tests are still failing.

Fixed

kianenigma
kianenigma reviewed Sep 20, 2022
View reviewed changes
Comment thread client/rpc/src/dev/tests.rs
kianenigma
kianenigma reviewed Sep 20, 2022
View reviewed changes
Comment thread client/service/test/src/client/mod.rs
@kianenigma kianenigma added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit. labels Sep 20, 2022
@kianenigma
Copy link
Copy Markdown
Contributor

kianenigma commented Sep 20, 2022

By the end of this PR, we should have no more use of decl_* in our codebase, so we should mark them as deprecated? @KiChjang WDYT?

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Sep 21, 2022

bot rebase

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot Bot commented Sep 21, 2022

Error: Command 'Command { std: "git" "merge" "origin/master" "--no-ff" "--no-edit", kill_on_drop: false }' failed with status Some(1); output: no output

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Sep 21, 2022

bot merge

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot Bot commented Sep 21, 2022

Waiting for commit status.

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot Bot commented Sep 21, 2022

Merge cancelled due to error. Error: Statuses failed for eee38f7

@kianenigma
Copy link
Copy Markdown
Contributor

kianenigma commented Sep 21, 2022

/tip small

@substrate-tip-bot
Copy link
Copy Markdown

substrate-tip-bot Bot commented Sep 21, 2022

@kianenigma A small tip was successfully submitted for koushiro (15XuanNimo5951s3RjFTPX1AvYVnCwfr3SDhb3AV4fQF3LpK on polkadot).

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

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Sep 22, 2022

bot merge

@paritytech-processbot
Copy link
Copy Markdown

paritytech-processbot Bot commented Sep 22, 2022

Waiting for commit status.

@paritytech-processbot paritytech-processbot Bot merged commit cc4d5cc into paritytech:master Sep 22, 2022
@koushiro koushiro deleted the remove-old-decl-macros branch September 22, 2022 08:56
@KiChjang
Copy link
Copy Markdown
Contributor

KiChjang commented Sep 23, 2022

By the end of this PR, we should have no more use of decl_* in our codebase, so we should mark them as deprecated? @KiChjang WDYT?

We already have a PR for that: #12326, although I must say that perhaps what we should really do is remove them rather than simply deprecating them.

ordian added a commit that referenced this pull request Sep 23, 2022
@ordian
Merge branch 'master' into ao-update-kvdb-and-co
45adc12 
* master:
  [Fix] parameter_types! dead code errors (#12340)
  [Feature] Sequential migration execution for try-runtime (#12319)
  bench: Use `_` instead  of `::` in auto-generated file names (#12332)
  Fast Unstake Pallet (#12129)
  Rename anonymous to pure proxy (#12283)
  Migrate remaining old decl_* macros to the new pallet attribute macros (#12271)
  pallet-utility: Disallow none origin (#12321)
  Make automatic storage deposits resistant against changing deposit prices (#12083)
  Format templates and fix `--steps` default value (#12286)
  Bump `wasmtime` to 1.0.0 (#12317)
  Introduce 'intermediate_insert' method to hide implementation details (#12215)
  Bound staking storage items (#12230)
  Use `array-bytes` for All Array/Bytes/Hex Operations (#12190)
  BREAKING: Rename Origin (#12258)
  Use temporary db for benchmarking (#12254)
  rpc: Implement `chainSpec` RPC API (#12261)
  Import target block body during warp sync (#12300)
  Proper naming wrt expectations (#12311)
  [ci] Revert cancel-pipeline job (#12309)
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@koushiro @ark0f
Migrate remaining old decl_* macros to the new pallet attribute macros (
3d1d774 
paritytech#12271)

* Migrate remaining old decl_* macros to the new pallet attribute macros

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Apply review suggestions

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Apply review suggestions

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* use pallet::storage

* Fix dev rpc test

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

* Fix service tests

Signed-off-by: koushiro <koushiro.cqx@gmail.com>

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@shawntabrizi shawntabrizi shawntabrizi left review comments

@kianenigma kianenigma kianenigma approved these changes

@bkchr bkchr bkchr approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D2-notlive 💤 PR contains changes in a runtime directory that is not deployed to a chain that requires an audit.

Projects

Runtime / FRAME
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@koushiro @bkchr @kianenigma @KiChjang @shawntabrizi