Pallets: Treasury deprecate propose_spend dispatchable
#14538
Conversation
muharem
added
A0-please_review
| @@ -346,6 +346,10 @@ pub mod pallet { | |||
| /// - O(1) | |||
| #[pallet::call_index(0)] | |||
| #[pallet::weight(T::WeightInfo::propose_spend())] | |||
| #[allow(deprecated)] | |||
There was a problem hiding this comment.
wonder if developers will notice these deprecations, may be there is a better way to deprecate dispatchables?
this at least will be included in release notes
There was a problem hiding this comment.
You mean the pr will be included? Otherwise no one really will see this from the upper layers.
There was a problem hiding this comment.
The metadata does not carry deprecation info, or?
There was a problem hiding this comment.
@bkchr yes, which is a good part.
the combination of #[allow(deprecated)] and #[deprecated(...)] for one item looks hacky.
may be whitelisting those warnings could be a solution.
@ggwpez just checked, it does not. you can see it in rococo contracts, the contracts pallet has deprecated calls.
but it does include docs, so we could include it into docs.
There was a problem hiding this comment.
There was a problem hiding this comment.
I created a deprecation issue for it - https://github.com/paritytech/substrate/issues/14732
Can you please review the PR
I did not add anything to metadata since there is no agreement on it yet.
| #[allow(deprecated)] | ||
| #[deprecated( | ||
| note = "`reject_proposal` will be removed in February 2024. Use `spend` instead." | ||
| )] | ||
| pub fn reject_proposal( |
There was a problem hiding this comment.
You could rename them to deprecated_reject_proposal. Then the info will be in the metadata and visible to downstream devs/users.
There was a problem hiding this comment.
it can be a solution actually, but I would first agree on it and have it as a standard to not create multiple ways of deprecating
|
I know that "Remove usage of the deprecated feature in the code base" is listed as a separate item in the deprecation process, but I think we should already use the new syntax instead of using |
But we have to keep the tests in tact for as long as the code is not completely removed, otherwise it could silentlybreak. |
|
but merge |
|
bot merge |
paritytech-processbot
bot
deleted the
muharem-treasury-deprecate-dispatchables
branch
* treasury deprecate dispatchables * allow deprecated * allow deprecated for benchmarks * allow deprecated in tests * allow deprecated for bounties tests * deprecation month
Deprecate
propose_spenddispatchable and it's dependant dispatchables,reject_proposalandapprove_proposal.The
spenddispatchable is expected to be used instead.Deprecation Issue: paritytech/polkadot-sdk#138