Remove all stale on_runtime_upgrade hooks in the runtime
#10650
Conversation
kianenigma
added
A0-please_review
| @@ -321,10 +321,6 @@ pub mod pallet { | |||
| Storage::<T>::process_deletion_queue_batch(weight_limit) | |||
| .saturating_add(T::WeightInfo::on_initialize()) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
@athei please confirm this is not used.
There was a problem hiding this comment.
How would I know if anyone uses this? It will migrate when appropriate. The code isn't there for fun.
There was a problem hiding this comment.
well this is exactly the reason why we are removing these hooks, because it is not clear who and when uses them.
My rephrased question is: are there any teams that you are aware of that would be affected if we remove this code?
note that I will re-expose this migration code as a standalone function, in case anyone needs it so they can manually apply it, and mark the PR is release-note.
There was a problem hiding this comment.
Its fine then. But I really much preferred the old way where it just worked. Won't I get all these dead code warnings after this change?
There was a problem hiding this comment.
You will see the outcome soon.
| @@ -316,13 +316,6 @@ pub mod pallet { | |||
| Unapproved, | |||
| } | |||
|
|
|||
| #[pallet::hooks] | |||
| impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> { | |||
| fn on_runtime_upgrade() -> frame_support::weights::Weight { | |||
There was a problem hiding this comment.
@hamidra please confirm this is not used.
There was a problem hiding this comment.
This is needed for the next statemine/statemint release to add ClassAccount storage to uniques.
Is this PR just a cleanup or we need to get rid of all migrations in the pallets?!
There was a problem hiding this comment.
Okay @hamidra you need to make sure that you we manually add this migration now to the next release of statemine. cc @joepetrowski
Might even be able to generate a compiler warning if it gets implemented inside the pallet 💡 |
|
Okay the warning's not possible without the unstable |
|
bot merge |
|
Waiting for commit status. |
| @@ -316,13 +316,6 @@ pub mod pallet { | |||
| Unapproved, | |||
| } | |||
|
|
|||
| #[pallet::hooks] | |||
| impl<T: Config<I>, I: 'static> Hooks<BlockNumberFor<T>> for Pallet<T, I> { | |||
| fn on_runtime_upgrade() -> frame_support::weights::Weight { | |||
There was a problem hiding this comment.
This is needed for the next statemine/statemint release to add ClassAccount storage to uniques.
Is this PR just a cleanup or we need to get rid of all migrations in the pallets?!
|
bot merge |
|
Waiting for commit status. |
|
Merge cancelled due to error. Error: Checks failed for edbe7a6 |
|
bot merge |
|
Waiting for commit status. |
|
Merge cancelled due to error. Error: Checks failed for 882992e |
…ove-stale-on-runtime-upgrades
…h#10650) * Remove all stale on_runtime_upgrade hooks in the runtime * add docs * cleanup * fix warn * fix more warnings * fix offence test * overwrite the damn UItest
…h#10650) * Remove all stale on_runtime_upgrade hooks in the runtime * add docs * cleanup * fix warn * fix more warnings * fix offence test * overwrite the damn UItest
and henceforth, we really try to not introduce them again, in the spirit of paritytech/polkadot-sdk#296 et. al. \
related but won't close #8713
This will also make the logs of
try-runtimemuch cleaner.