[Meta/PM] Create a deprecation process

[juangirini]

There is a need to define a feature deprecation process.
We can use this issue for a discussion to define the process.

Deprecation process

Create a 'deprecation' labeled issue with the following steps:

1. Hard deprecation by adding a warning message *
2. Remove usage of the deprecated feature in the code base
3. Update examples and tutorials
4. Deprecation is noted in the release log
5. Removal version is planned
6. Deprecated feature is removed
7. Removal of the feature is noted in the release log

---

* The warning message should include a removal month and year which is suggested to be 6 months from the deprecation notice is released. This means that the feature will be removed in a release within that month (or shortly after, but never before).
E.g.

#[deprecated(note = "GenesisConfig is planned to be removed in December 2023. Use `RuntimeGenesisConfig` instead.")]

[juangirini]

I want to link @kianenigma's comment to add a bit of context

[ggwpez]

We probably also need a tracking issue for all the deprecated things that we have.
Could you maybe start one with a checklist or table @juangirini?

The two things from Kians comment obviously. Otherwise I can think of: Weight::{from_ref_time, from_proof_size}, other V1 weight stuff (1, 2).
Just generally checking the code for deprecated can also help.

[KiChjang]

We can also talk about the deprecated code that I left behind as part of refactoring all the Weight related types and logic into sp_weight. They're largely located in https://github.com/paritytech/substrate/blob/master/frame/support/src/weights.rs, where you can see there's a lot of deprecated items, with helpful hints as to where the actual types have been moved to.

A couple of observations to note:

1. Types that still uses the old path, e.g. frame_support::weight::DispatchClass, are simply type aliases to the type now located in the new path (frame_support::dispatch::DispatchClass).
2. Functions do not support aliasing, so instead what happens is that we create a new function in the old path (frame_support::weight::extract_actual_weight), and in its body, we call the function that's now located in the new path (frame_support::dispatch::extract_actual_weight).
3. Deprecating traits is a bit trickier, but not impossible. The way I handled it is by making the relocated trait a supertrait of the old trait path, e.g. frame_support::weight::OneOrMany is the old path, so now it has frame_support::dispatch::OneOrMany as its supertrait.

These are all measures to ensure that code that hasn't migrated yet would still work as intended, but would generate a deprecation warning during every compilation. I would suggest that this going forward, we should look into trying to support such a deprecation model as it allows users to migrate code at their leisure.

However, what still requires discussion is how long we intend to keep the deprecated code around until we remove them altogether.

[juangirini]

@ggwpez @KiChjang we now have this table to follow up with 'deprecation' related issues. Please feel free to create new issues with the 'deprecation' keyword in the title to add them in there

[aaronbassett]

A deprecation process is a good step in the right direction, but it needs a defined release process so that users know when "Later: total removal" will occur.

If we adopted semantic versioning major.feature.patch, deprecation could occur during any feature release version. Still, the removal wouldn't occur until the next major release or the first feature release version of a new major release if the deprecation occurs in the last feature release version of the previous major release. This is to ensure that deprecation warnings are present in at least two versions before removal.

For example:

• A feature is deprecated in 6.4, the next release is 7.0, and the feature is removed in 7.1
• Or a feature is deprecated in 8.2, it is still available for 8.3, 8.4, 8.5, and so on, before being removed in 9.0

Using this as a basis, I would imagine the process would then be as follows:

1. Decision is made to deprecate a feature in the next feature release
2. Docs, tutorials, templates, VODs, etc are updated to remove references to deprecated feature
3. Compiler deprecation warnings are added
4. Feature release version number is bumped
5. Deprecation is noted in the release log
6. Feature release version is published at the same time as updated docs, etc
7. Major release is planned
8. Deprecated feature is removed from the code base
9. Removal of the feature is noted in the release log
10. New major version is published

[juangirini]

A feature is deprecated in 6.4, the next release is 7.0, and the feature is removed in 7.1

If we adopt semantic versioning, then we can't introduce a breaking change in a minor release. As explained here, in your example 6.4 would include the deprecation warning and 7.0 the actual removal.

Nevertheless, I don't think we are anywhere close to adopt semver, and I think we should introduce a deprecation process before that. We can always revisit the process after implementing semver.

Other than that the flow you propose seems logical, and I think that if we find a compromise solution i those lines where we don't need semver we should be good.

However, what still requires discussion is how long we intend to keep the deprecated code around until we remove them altogether.

Until we don't implement semantic versioning we might need to find a solution that takes time and releases into account.

My suggestion is to remove a feature in the first release after 6 months have passed since the release in which the feature was notified as 'deprecated' in its notes.

[aaronbassett]

I probably shouldn't have called it semantic versioning; that was lazy shorthand on my part. What I really meant was a clearly defined versioning scheme. Of course, we could just adhere to semver to avoid confusion, I just dislike the "at least one", it seems too short IMHO.

Before you completely remove the functionality in a new major release there should be at least one minor release that contains the deprecation so that users can smoothly transition to the new API.

This is why I was suggesting a minimum of two releases which contain the deprecation, but that puts us in the position of potentially introducing a backward incompatible change on a non-major release.

My suggestion is to remove a feature in the first release after 6 months have passed since the release in which the feature was notified as 'deprecated' in its notes.

Do we have any stats on how quickly people migrate between versions? Can we view downloads/installations by version number currently?

[juangirini]

Those are very good questions, I'll reach out to the Data Team to see if they can help us with some stats

[ggwpez]

Depending on what process you settle here, the decl macro deprecation would also be a good candidate: paritytech/substrate#12248
Would like to delete it ASAP since it blocks a ton of other issues.

[juangirini]

I will take #175 as a guinea pig for the process as it is a simple task.
The process that I am going to try is

• Create a 'deprecation' labeled issue with the following
• Soft deprecation and removal from the codebase
• Update examples and tutorials
• Hard deprecation by adding a deprecate compiler warning
• Deprecation is noted in the release log
• Removal version is planned -> we don't know yet how much time (how many releases) to wait, but we can test it
• Deprecated feature is removed from the code base
• Removal of the feature is noted in the release log

[gpestana]

A few notes/ideas about the deprecation process from an extrinsics perspective, as a continuation of a few offline discussions we have had.

Currently, there is also no clear process to follow to deprecate or change the signature of extrinsics. Usually, we start by reaching out the community about the change and decide what is the best deprecation/update path. Communication is crucial since removing or refactoring extrinsics may break dapps (e.g. wallets with staking when the controller account is deprecated) and teams that do not keep up with the forum will only realise about the deprecation when the their app breaks.

One idea that has been floating around is to include in the metadata information about the extrinsics that are marked as deprecated and use that as a communication channel for projects to learn about soon-to-be deprecated or refactored extrinsics. The advantage is that community have an easier way to "subscribe" to changes in the extrinsics they depend, provided that they consume the metadata correctly. On the other hand, the disadvantage is that it bloats up the metadata, but the increase in storage requirements should be pretty low.

Another example of where this feature would be useful when we end up deprecating the chill_other extrinsic in staking, which we will postpone due to the potential breaking changes in the ecosystem apps. A feature like this would give us more confidence that we can do these types of changes with less friction.

Implementation ideas

• We could annotate extrinsics with #[pallet::deprecate("substrate/issues/XXXX")] and then the metadata amalgamator extracts all the deprecation annotations and includes it in the extrinsic metadata with the issue referring with deprecation notes.

• Another options would be only annotate with #[pallet::deprecate)] and rely on the extrinsic comments to add deprecation details and pointers.

There is a breaking change coming up in with Metadata V15, perhaps we could piggy back this change in the release (~end of June).

[Polkadot-Forum]

This issue has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/stablising-v15-metadata/2819/9

[xlc]

@gpestana I like the idea. However, I don't really want to overload the metadata format.

I had this idea since very beginning. How about just support custom metadata?
Allow something like #[pallet::metadata(name, value)] to be attached to extrinsics (and storages) and SDK can read those value and do whatever they want.

In that way we can extend metadata without extend the metadata format.

For this case, it could be #[pallet::metadata(known_metadata_key::DEPRECATE", "since version 0.9.43")]

[juangirini]

@gpestana that's a great idea, and @xlc suggestion is a very nice way to generalise it.
The downside of the last proposal is that is less explicit, and a "DEPRECATE" key within "metadata" is more likely to be ignored if not understood by the reader. But not sure if that's a big deal.
@gpestana do you see any other pros of having a dedicated deprecate key instead of a generic on within metadata?

I really like the #[pallet::deprecate("")] syntax, which could actually be syntax sugar that translates into #[pallet::metadata("","")] behind the scenes.

I see this "metadata" solution has been proposed here.

[gpestana]

I really like the #[pallet::deprecate("")] syntax, which could actually be syntax sugar that translates into #[pallet::metadata("","")] behind the scenes.

Yes, I think this would be a good approach -- we can make the deprecation annotation explicit in the code without bloating the metadata format.

[juangirini]

I want to bring in the "deprecation" side-discussion started on paritytech/substrate#12939.

@bkchr has some concerns about the generic metadata proposed by @xlc that can be used for deprecation notices.

TBH, just sounds like a band tape on the shitty process we have. This should go into the release notes. No need to put this kind of stuff into the metadata where people will also just ignore it.

It is indeed a band tape, and we are trying to come up with a compromise solution while we don't have proper semver in place.
I think we all agree that we need release notes, and more visibility on deprecation/breaking changes.

And I also agree with @gpestana on this:

allowing developers to "subscribe" to deprecation through metadata could be a good complement to the process

[gpestana]

It seems to me that it will be a stretch to add the deprecated info to the V15 release, but I think we could keep the discussion going and work on this to eventually add this feature to the next metadata format release.

[juangirini]

1. Create a 'deprecation' labeled issue with the following
2. Soft deprecation and removal from the codebase
3. Update examples and tutorials
4. Hard deprecation by adding a warning message *
5. Deprecation is noted in the release log
6. Removal version is planned
7. Deprecated feature is removed from the code base
8. Removal of the feature is noted in the release log

What do you think about removing step 2 and replace it by step 4? So that there is no soft deprecation.
Currently, step 3 needs to happen after step 2, and step 4 needs to happen after 3. Whereas this seems ideal for developers, it slows down the process quite a lot.
This new proposal would allow async work between FRAME and docs teams and therefore a faster process.

[juangirini]

As I could understand from all related threads, there is no decision on whether we should include the deprecation note into the metadata or not.

A simple approach for now could be to just add the _deprecated suffix to the dispatchable.

One simpler decision that we could do, it is a pattern/suffux for PR titles or/and a label (we could include this info into the issue template with the checklists). This will help with release notes and search.

I like this idea, what would you suggest?

[rzadp]

there is no decision on whether we should include the deprecation note into the metadata or not.

As a dApp/tooling developer, I wanted to give my perspective and give arguments for including deprecation notes in the metadata. I'm bumping this issue because I didn't find any other.

It could be just a reiteration of the points that @gpestana made. As a developer that had been affected by this very change and others, I agree with those points.

---

sounds like a band tape on the shitty process we have.

@bkchr My perspective is different. Even if a perfect process existed and was implemented (with release logs, announcements, long deprecation period, migration docs), that could be not enough, because for example:

• A developer has read the release logs / announcement, but fails to realize that a method that's being deprecated is used in the code of one of many apps he supports.
• A developer has read the release logs / announcement, but only after some time he takes over support of an affected project and too much time has passed to connect the dots.

Therefore I don't see deprecation notes in metadata as a band tape or replacement, but as an additional enablement of automation. An automation I could do right now, in all my apps, if only a deprecation text existed in metadata:

# I'm talking about TypeScript apps using PJS at this time.
# I assume it could be similar with PAPI in the future and with Rust and subxt,
# although I don't have experience with that.

- Add a JS Proxy to appropriate PJS objects.
- Whenever an extrinsic or query is proxied:
  - Read the corresponding metadata.
  - If a deprecation note is found:
    - If it's in production, do a `console.warn`, or increase a prometheus warning metric, or whatever fits.
    - If it's in CI, throw an error and make the E2E tests (using a local polkadot-sdk dev node) red.

For my use-cases, a deprecation note could be as simple as a Deprecated: <note> string in the metadata docs (so possible to do now, even without v15 Custom Metadata).

Please note that this is possible to do without any upstream changes to PJS. And it would detect and alarm me automatically, even without rebuilding the code or upgrading dependencies.

AFAIK there is no other way to achieve it, if deprecation info is not included in the metadata (well other than somehow annotating JSON-RPC with deprecation info).

I'm not proficient with Rust or polkadot-sdk codebase, but I've dived into it to gain some understanding, saw the preprocessing of Pallets' calls metadata, and it seems to me like it's doable to enforce that: if a #[deprecated] attribute is there, a deprecated text also exists in the docs (or automatically append it). I could try to contribute it with a PR.

---

To sum up:

• I think deprecation notes in metadata are desirable as a complement to the release process.
• I think they would enable automations that are not possible without them.
• Since no specific metadata fields (or Custom Metadata keys) are defined yet, I propose the deprecation note to be a simple human-readable string in the docs that a machine can parse as well.
• I can try to amateurishly contribute a PR with the above.

cc @kianenigma who I've seen working with those docs recently.

[kianenigma]

In general I am not against this and find it useful.

if a #[deprecated] attribute is there, a deprecated text also exists in the docs (or automatically append it).

My only comment is that I think this should not be a random text string in the doc and therefore metadata, but instead it should be a filed in the metadata for calls, events, errors and storage items.

There will indeed be a cost to this, as a lot of deprecated?: false needs to be appended everywhere, but I suppose we can make it worthwhile with a convention where the lack of this field means it is not deprecated. Or, what @xlc proposed here

Please make a new issue for this, add the FRAME label, and optionally add it to the developer wishlist project column as well, where I am trying to track similar issues.

@jsdw wdyt?

[rzadp]

Please make a new issue for this, add the FRAME label, and optionally add it to the developer wishlist project column as well, where I am trying to track similar issues.

@kianenigma Done, created this issue and added to the wishlist.