Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Deprecation info support in RuntimeMetadataIR #4851

Merged
merged 43 commits into from
Sep 11, 2024
Merged

Deprecation info support in RuntimeMetadataIR #4851

merged 43 commits into from
Sep 11, 2024

Conversation

pkhry
Copy link
Contributor

@pkhry pkhry commented Jun 20, 2024 •
edited
Loading

Description:

  • Adds DeprecationStatusIR enum to sp_metadata_ir.
    Deprecation info for simple items.
  • Adds DeprecationInfoIR enum to sp_metadata_ir.
    It is a deprecation info for an enums/errors/calls. Contains DeprecationStatusIR.
    Denotes full/partial deprecation of the type or its variants/calls
  • Adds deprecation_info field to
    - RuntimeApiMetadataIR
    - RuntimeApiMethodMetadataIR
    - StorageEntryMetadataIR
    - PalletConstantMetadataIR
    - PalletCallMetadataIR
    - PalletMetadataIR
    - PalletEventMetadataIR
    - PalletErrorMetadataIR

Testing done:

  • Unit tests to check whether or not correct note/since texts are getting propagated to the metadata structs.
  • UI test to check for error message in case of incorrect attribute usage.
    There's also some test updates to make sure that deprecation attributes are getting propagated to the relevant structs.

see: #4098, Solution: A

Examples of produced deprecation info metadata

They can be found in:

@lexnv
Copy link
Contributor

lexnv commented Jun 26, 2024

Nice work here @pkhry! Certainly off to a good start! 👍

Left a few thoughts after the initial review, mainly:

  • would be good to keep the changes related to this PR only (ie adjust the rustfmt)
  • a few more tests to illustrate how this feels in practice for the user experience (also to validate the approach for all deprecation included calls / storage etc)
  • would be interesting also to include a negative test that fails to compile because pallet::deprecated includes unsupported fields (ie fields that are not empty; "note" or "since")
  • make the CI happy (to double check the tests for now, everything should pass since changes are made in isolation)

From the CI step: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6513912:

error: unused import: `proc_macro::TokenStream`
  --> substrate/frame/support/procedural/src/pallet/parse/config.rs:20:5
   |
20 | use proc_macro::TokenStream;
   |     ^^^^^^^^^^^^^^^^^^^^^^^

After that we can have a look at exposing deprecation info for events / errors (maybe in a separate PR), your initial approach of using a BTreeMap sounds good offhand, but I would consolidate this first. Let me know if you have any questions here 🙏

@lexnv lexnv assigned lexnv and unassigned lexnv Jun 27, 2024
@lexnv lexnv added R0-silent Changes should not be mentioned in any release notes I5-enhancement An additional feature request. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Jun 27, 2024
@pkhry pkhry force-pushed the pkhry/deprecation_attr_frame_support branch from f0008e0 to c8d2285 Compare June 27, 2024 23:06
@pkhry pkhry requested a review from lexnv June 28, 2024 00:01
@pkhry pkhry marked this pull request as ready for review June 28, 2024 00:01
@pkhry
Copy link
Contributor Author

pkhry commented Sep 5, 2024

bot fmt

@command-bot
Copy link

command-bot bot commented Sep 5, 2024 •
edited
Loading

@pkhry https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7266675 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 28-9f055b1f-93d4-483a-bdfb-2b324e416638 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@pkhry Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7266675 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7266675/artifacts/download.

@pkhry pkhry requested a review from bkchr September 5, 2024 09:37
@pkhry
Copy link
Contributor Author

pkhry commented Sep 5, 2024

bot fmt

@command-bot
Copy link

command-bot bot commented Sep 5, 2024 •
edited
Loading

@pkhry https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7268914 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 30-8a8a688b-538e-49a8-9729-b9b8f9ab164e to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 5, 2024

@pkhry Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7268914 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7268914/artifacts/download.

@pkhry
Copy link
Contributor Author

pkhry commented Sep 6, 2024

/cmd update-ui

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 6, 2024

Command "update-ui" has started 🚀 See logs here

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 6, 2024

Command "update-ui" has failed ❌! See logs here

@pkhry
Copy link
Contributor Author

pkhry commented Sep 6, 2024

bot update-ui

@command-bot
Copy link

command-bot bot commented Sep 6, 2024 •
edited
Loading

@pkhry https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7285661 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/update-ui/update-ui.sh". Check out https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 2-224eee01-9ce2-499b-a991-e0a0763c2564 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Sep 6, 2024

@pkhry Command "$PIPELINE_SCRIPTS_DIR/commands/update-ui/update-ui.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7285661 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/7285661/artifacts/download.

@pkhry
Copy link
Contributor Author

pkhry commented Sep 8, 2024

/cmd update-ui

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 8, 2024

Command "update-ui" has started 🚀 See logs here

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 8, 2024

Command "update-ui" has failed ❌! See logs here

@bkchr bkchr added this pull request to the merge queue Sep 11, 2024
Merged via the queue into master with commit ec9a734 Sep 11, 2024
182 of 204 checks passed
@bkchr bkchr deleted the pkhry/deprecation_attr_frame_support branch September 11, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gui1117 gui1117 gui1117 approved these changes

@kianenigma kianenigma kianenigma approved these changes

@bkchr bkchr bkchr approved these changes

@lexnv lexnv lexnv approved these changes

Assignees
No one assigned
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I5-enhancement An additional feature request. R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@pkhry @lexnv @kianenigma @paritytech-cicd-pr @Polkadot-Forum @bkchr @gui1117 @actions-user