Skip to content

remove pallet::getter from pallet-offences#6027

Merged
Zebedeusz merged 7 commits into
masterfrom
zebedeusz/remove_getter_pallet_offences
Oct 16, 2024
Merged

remove pallet::getter from pallet-offences#6027
Zebedeusz merged 7 commits into
masterfrom
zebedeusz/remove_getter_pallet_offences

Conversation

@Zebedeusz

@Zebedeusz Zebedeusz commented Oct 11, 2024

Copy link
Copy Markdown
Contributor

Description

Part of #3326
Removes pallet::getter from pallet-offences from type Reports.
Adds a test to verify that retrieval of Reports still works with storage::getter.

polkadot address: 155YyFVoMnv9BY6qxy2i5wxtveUw7WE1n5H81fL8PZKTA1Sd

@Zebedeusz Zebedeusz added the T2-pallets This PR/Issue is related to a particular pallet. label Oct 11, 2024
@Zebedeusz Zebedeusz requested a review from a team as a code owner October 11, 2024 12:45
@Zebedeusz Zebedeusz force-pushed the zebedeusz/remove_getter_pallet_offences branch from 6d121c6 to b74ddc3 Compare October 11, 2024 12:48
Comment thread prdoc/pr_6027.prdoc
Comment thread substrate/frame/offences/src/tests.rs
@shawntabrizi

Copy link
Copy Markdown
Member

Yeah, we need you to create a new public function which does the same thing as the getter here.

@Zebedeusz Zebedeusz requested review from a team as code owners October 14, 2024 10:12
@paritytech-review-bot paritytech-review-bot Bot requested a review from a team October 14, 2024 10:13
@Zebedeusz Zebedeusz marked this pull request as draft October 14, 2024 10:15
@Zebedeusz Zebedeusz force-pushed the zebedeusz/remove_getter_pallet_offences branch from 92b3ceb to 6c2895b Compare October 14, 2024 10:25
@Zebedeusz Zebedeusz marked this pull request as ready for review October 14, 2024 10:27
Comment thread substrate/frame/offences/src/lib.rs Outdated
Comment thread substrate/frame/offences/src/lib.rs Outdated
Comment thread substrate/frame/offences/src/lib.rs Outdated
Comment thread substrate/frame/offences/src/lib.rs Outdated
>;

impl<T: Config> Pallet<T> {
#[deprecated(note = "Use Reports::<T>::get(id) instead.")]

@shawntabrizi shawntabrizi Oct 14, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

but there is no need to deprecate here.

Just keep this public function around forever. The point is just to remove the macro code/logic, not necessarily remove this functionality.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah that was my request - that was the impression that I got from the discussion on the issue, maybe I'm not up to date on the intention behind removing the getters

@shawntabrizi

Copy link
Copy Markdown
Member

I think we can approved after you integrate our final feedback here.

thanks!

@shawntabrizi

Copy link
Copy Markdown
Member

Please include your polkadot address in your pr description: https://github.com/paritytech/substrate-tip-bot

@github-actions github-actions Bot requested a review from seadanda October 15, 2024 10:05
@github-actions

Copy link
Copy Markdown
Contributor

Review required! Latest push from author must always be reviewed

@Zebedeusz

Copy link
Copy Markdown
Contributor Author

Alright, everything done from your feedback, I'd say. Thanks guys!

Comment on lines +154 to +160
/// Get the offence details from reports of given ID.
pub fn reports(
report_id: ReportIdOf<T>,
) -> Option<OffenceDetails<T::AccountId, T::IdentificationTuple>> {
Reports::<T>::get(report_id)
}

@shawntabrizi shawntabrizi Oct 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

great, this is really all you need to do :)

Looking forward to seeing more contributions

Comment thread substrate/frame/offences/src/tests.rs Outdated
Comment thread substrate/frame/offences/src/tests.rs Outdated
@shawntabrizi

Copy link
Copy Markdown
Member

/tip small

@substrate-tip-bot

Copy link
Copy Markdown

@shawntabrizi A referendum for a small (20 DOT) tip was successfully submitted for @Zebedeusz (155YyFVoMnv9BY6qxy2i5wxtveUw7WE1n5H81fL8PZKTA1Sd on polkadot).

Referendum number: 1231.
tip

@Zebedeusz Zebedeusz enabled auto-merge October 15, 2024 14:14
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.

4 participants