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

removed pallet::getter from example pallets #3371

Merged
merged 10 commits into from
Feb 21, 2024
Merged

removed pallet::getter from example pallets #3371

merged 10 commits into from
Feb 21, 2024

Conversation

muraca
Copy link
Contributor

@muraca muraca commented Feb 17, 2024 •
edited
Loading

part of #3326

@ggwpez @kianenigma @shawntabrizi

polkadot address: 12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp

@muraca
removed pallet::getter from pallet-example-basic
071236c 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca
removed pallet::getter from pallet-example-kitchensink
e66ca71 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca
removed pallet::getter from pallet-example-offchain-worker
e11f534 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca muraca requested a review from a team as a code owner February 17, 2024 10:31
@muraca
removed pallet::getter from pallet-template
9293f69 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca muraca mentioned this pull request Feb 17, 2024
Open
63 tasks
ggwpez
ggwpez reviewed Feb 17, 2024
View reviewed changes
substrate/bin/node-template/pallets/template/src/tests.rs Outdated
@@ -9,7 +9,7 @@ fn it_works_for_default_value() {
// Dispatch a signed extrinsic.
assert_ok!(TemplateModule::do_something(RuntimeOrigin::signed(1), 42));
// Read pallet storage and assert an expected result.
assert_eq!(TemplateModule::something(), Some(42));
assert_eq!(<Something<Test>>::get(), Some(42));
Copy link
Member

Choose a reason for hiding this comment

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

Im not sure if we settled on a preferred syntax, this turbofish vs ::.
Personally i think :: everywhere is better, but without linter it will probably not work out anyway.

Copy link
Member

Choose a reason for hiding this comment

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

I agree that Something::<Test>::get() is preferred

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes and yes.

kianenigma
kianenigma approved these changes Feb 19, 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.

@muraca can you in general in this migration encourage the use of Type::<T>::foo() over <Type<T>>::foo()?

@muraca
Copy link
Contributor Author

muraca commented Feb 19, 2024 •
edited
Loading

I will, but keep in mind that it's going to be tough to enforce without some kind of linting

@kianenigma
Copy link
Contributor

I will, but keep in mind that it's going to be tough to enforce without some kind of linting

Let's do the best we can in manual reviews, but indeed it is not a strict rule. Thanks!

kianenigma
kianenigma reviewed Feb 19, 2024
View reviewed changes
substrate/bin/node-template/pallets/template/src/tests.rs
@@ -1,4 +1,4 @@
use crate::{mock::*, Error, Event};
use crate::{mock::*, Error, Event, Something};
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update https://github.com/substrate-developer-hub/substrate-node-template as well? see #3155

@kianenigma kianenigma added T13-deprecation The current issue/pr is, or should be, part of a deprecation process. R0-silent Changes should not be mentioned in any release notes labels Feb 19, 2024
@kianenigma kianenigma added this pull request to the merge queue Feb 19, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Feb 19, 2024
@kianenigma kianenigma removed the R0-silent Changes should not be mentioned in any release notes label Feb 19, 2024
@muraca
removed pallet::getter from pallet-parachain-template
a231d6c 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca
remove turbofish syntax usage
9d21e58 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca
fixed a replace all mistake
4c4f146 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca muraca changed the title removed pallet::getter from Substrate example pallets removed pallet::getter from example pallets Feb 19, 2024
@muraca
prdoc for paritytech#3371
835ae80 
Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@muraca
last typo (hopefully)
6c28792 
at least you have the proof I'm not an AI

Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
@bkchr bkchr added this pull request to the merge queue Feb 21, 2024
Merged via the queue into paritytech:master with commit cd91c6b Feb 21, 2024
130 checks passed
@liamaharon
Copy link
Contributor

/tip small

@substrate-tip-bot substrate-tip-bot
Copy link

@liamaharon A referendum for a small (20 DOT) tip was successfully submitted for @muraca (12poSUQPtcF1HUPQGY3zZu2P8emuW9YnsPduA4XG3oCEfJVp on polkadot).

Referendum number: 578.
tip

@substrate-tip-bot substrate-tip-bot
Copy link

The referendum has appeared on Polkassembly.

@rzadp rzadp mentioned this pull request Mar 15, 2024
Open
bgallois pushed a commit to duniter/duniter-polkadot-sdk that referenced this pull request Mar 25, 2024
@muraca @kianenigma
removed pallet::getter from example pallets (paritytech#3371)
059632d 
part of paritytech#3326 

@ggwpez @kianenigma @shawntabrizi

---------

Signed-off-by: Matteo Muraca <mmuraca247@gmail.com>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
This was referenced Jun 5, 2024
Closed
Closed
Closed
Closed
@ayushmishra2005 ayushmishra2005 mentioned this pull request Oct 21, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kianenigma kianenigma kianenigma approved these changes

@shawntabrizi shawntabrizi shawntabrizi left review comments

@ggwpez ggwpez ggwpez approved these changes

@bkchr bkchr bkchr approved these changes

Assignees
No one assigned
Labels
T13-deprecation The current issue/pr is, or should be, part of a deprecation process.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@muraca @kianenigma @liamaharon @shawntabrizi @bkchr @ggwpez