-
Notifications
You must be signed in to change notification settings - Fork 666
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 from pallet-offences #6027
base: master
Are you sure you want to change the base?
Conversation
6d121c6
to
b74ddc3
Compare
|
||
crates: | ||
- name: pallet-offences | ||
bump: patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the getter is a breaking (major) change unless you add a function with the same signature. I'd go with adding the new function but marking it as deprecated to avoid the breakage but letting people know that it is deprecated and that they should use Reports::<T>::get(id)
instead.
// when | ||
let stored_offence_details = Reports::<Runtime>::get(report_id); | ||
|
||
// then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// when | |
let stored_offence_details = Reports::<Runtime>::get(report_id); | |
// then | |
#[allow(deprecated)] | |
let stored_offence_details = Offences::reports(report_id); | |
assert_eq!(stored_offence_details, Some(offence_details.clone())); | |
let stored_offence_details = Reports::<Runtime>::get(report_id); |
This new part of the test should pass to avoid a breaking change
Yeah, we need you to create a new public function which does the same thing as the getter here. |
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.