-
Notifications
You must be signed in to change notification settings - Fork 745
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 the bounties and child-bounties pallets #4392
Remove pallet::getter usage from the bounties and child-bounties pallets #4392
Conversation
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.
I would prefer in such case to provide a pub getters for storage items and have a minor bump for the crate.
These pallets are old, if someone using those public getters to read data, this will be a breaking change, and it will take some time for maintainers to get those getters back. It wont be a nice experience.
@bkchr @ggwpez what you think?
Also storage items are part of public API of a pallet, even if there are no getter for them. So I believe additional getters do not add much to maintenance. |
Is there some way that introducing these changes breaks systems in production? I was thinking people would just need to update their codebase if they depended on the getters, but not familiar with every part of this. |
Yeah that is the case. Generally I don't know if anyone build something on top of these crates and used these getters, but also the release notes are quite clear. Still a better way would have been to first deprecate these getters and then to remove them. |
Happy to deprecate first moving forward if that is preferred. Just let me know 👍 |
For me it is fine to remove them. This is not the first PR doing this, so it would be weird to stop now 🙈 |
The storage items actually are public. They can be accessed from outside of the pallet without getter. Sorry for confusion. |
6487ac1
…ets (paritytech#4392) As per paritytech#3326, removes pallet::getter usage from the bounties and child-bounties pallets. The syntax `StorageItem::<T, I>::get()` should be used instead. Changes to one pallet involved changes in the other, so I figured it'd be best to combine these two. cc @muraca --------- Co-authored-by: Bastian Köcher <git@kchr.de>
…ets (paritytech#4392) As per paritytech#3326, removes pallet::getter usage from the bounties and child-bounties pallets. The syntax `StorageItem::<T, I>::get()` should be used instead. Changes to one pallet involved changes in the other, so I figured it'd be best to combine these two. cc @muraca --------- Co-authored-by: Bastian Köcher <git@kchr.de>
As per #3326, removes pallet::getter usage from the bounties and child-bounties pallets. The syntax
StorageItem::<T, I>::get()
should be used instead.Changes to one pallet involved changes in the other, so I figured it'd be best to combine these two.
cc @muraca