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

Remove pallet::getter usage from authority-discovery pallet #4091

Merged

Conversation

PolkadotDom
Copy link
Contributor

As per #3326, removes pallet::getter usage from the pallet authority-discovery. The syntax StorageItem::<T, I>::get() should be used instead.

cc @muraca

@PolkadotDom PolkadotDom requested a review from a team as a code owner April 12, 2024 00:55
@cla-bot-2021
Copy link

cla-bot-2021 bot commented Apr 12, 2024

User @PolkadotDom, please sign the CLA here.

@muraca
Copy link
Contributor

muraca commented Apr 15, 2024

Copy link

Review required! Latest push from author must always be reviewed

@liamaharon liamaharon added the T2-pallets This PR/Issue is related to a particular pallet. label Apr 16, 2024
prdoc/pr_4091.prdoc Outdated Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 16, 2024 13:19
@PolkadotDom
Copy link
Contributor Author

@liamaharon resolved the previous comment if you have a second for re-review

@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 23, 2024 16:27
prdoc/pr_4091.prdoc Outdated Show resolved Hide resolved
@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 29, 2024 05:58
@PolkadotDom PolkadotDom requested a review from muharem May 6, 2024 17:24
@PolkadotDom
Copy link
Contributor Author

Realized I need to implement getters for the storage values since they were previously public. Hold on review for the moment.

@muharem
Copy link
Contributor

muharem commented May 7, 2024

@PolkadotDom I did not know they were public, good catch. when you introduce them, these changes wont be breaking anymore, you can change bump to minor

@PolkadotDom
Copy link
Contributor Author

PolkadotDom commented May 8, 2024

@PolkadotDom I did not know they were public, good catch. when you introduce them, these changes wont be breaking anymore, you can change bump to minor

Sounds good! 👍

@PolkadotDom
Copy link
Contributor Author

@muharem Actually, it looks like there was a bit of a redundancy. The pallet previously had both macro generated getters as well as explicit getters. So instead of implementing new ones for the lost macros I'm just going to keep the bump major and update the prdoc to reflect that the explicit getters should be used moving forward. Let me know if you think it should be otherwise.

@github-actions github-actions bot requested a review from liamaharon May 8, 2024 01:14
@bkchr bkchr enabled auto-merge May 10, 2024 21:01
@bkchr bkchr added this pull request to the merge queue May 10, 2024
Merged via the queue into paritytech:master with commit 32deb60 May 10, 2024
147 of 148 checks passed
ordian added a commit that referenced this pull request May 14, 2024
* master:
  improve MockValidationDataInherentDataProvider to support async backing (#4442)
  Bump `proc-macro-crate` to the latest version (#4409)
  [ci] Run check-runtime-migration in GHA (#4441)
  prospective-parachains rework (#4035)
  [ci] Add forklift to GHA ARC (#4372)
  `CheckWeight` SE: Check for extrinsic length + proof size combined (#4326)
  Add generate and verify logic for `AncestryProof` (#4430)
  Rococo AH: undeploy trie migration (#4414)
  Remove `substrate-frame-cli` (#4403)
  migrations: `take()`should consume read and write operation weight (#4302)
  `remote-externalities`: store block header in snapshot (#4349)
  xcm-emlator: Use `BlockNumberFor` instead of `parachains_common::BlockNumber=u32` (#4434)
  Remove `pallet::getter` usage from authority-discovery pallet (#4091)
  Remove pallet::getter usage from pallet-contracts-mock-network (#4417)
  Add docs to request_core_count (#4423)
@PolkadotDom PolkadotDom deleted the dom/remove-getter-pallet-authority-discovery branch May 14, 2024 21:13
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
…tech#4091)

As per paritytech#3326, removes pallet::getter usage from the pallet
authority-discovery. The syntax `StorageItem::<T, I>::get()` should be
used instead.

cc @muraca

---------

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
…tech#4091)

As per paritytech#3326, removes pallet::getter usage from the pallet
authority-discovery. The syntax `StorageItem::<T, I>::get()` should be
used instead.

cc @muraca

---------

Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants