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.

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

Moves runtime macro out of experimental flag #4249

Merged
merged 4 commits into from
May 29, 2024
Merged

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented Apr 23, 2024

Step in #3688

Now that the runtime macro (Construct Runtime V2) has been successfully deployed on Westend, this PR moves it out of the experimental feature flag and makes it generally available for runtime devs.

@gupnik gupnik added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Apr 23, 2024
@gupnik gupnik requested a review from a team as a code owner April 23, 2024 05:06
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally not sure if just because it was deployed on one network, it should be taken out of the experimental flag? Given this argument, we would have never needed to add it there in the first place, because no downstream user had even the time to try this out.

prdoc/pr_4249.prdoc Outdated Show resolved Hide resolved
Co-authored-by: Bastian Köcher <git@kchr.de>
@paritytech-review-bot paritytech-review-bot bot requested a review from a team April 23, 2024 09:08
@gupnik
Copy link
Contributor Author

gupnik commented Apr 23, 2024

Generally not sure if just because it was deployed on one network, it should be taken out of the experimental flag? Given this argument, we would have never needed to add it there in the first place, because no downstream user had even the time to try this out.

Yeah, makes sense. @ggwpez had suggested to put in under the experimental flag to make sure that the metadata is not impacted after the change. I felt that this has been validated now but happy to wait in case you feel something else is needed first.

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which test networks are using this now?

I have less of a strong opinion on letting it remain experimental, probably would opt for the safer option of letting it be there for now.

Instead, I would put the focus on making sure this is used as much as possible within our codebase/templates.

@gupnik
Copy link
Contributor Author

gupnik commented Apr 26, 2024

Which test networks are using this now?

Currently on westend.

Instead, I would put the focus on making sure this is used as much as possible within our codebase/templates.

Yeah, my hope was to add it in all test runtimes, but the experimental flag makes it difficult.

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the feature = experimental has very limited use in this context, maybe we should not use it at all. But that is OT and this should not remain blocked.

Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me to move it out of experimental.
We could always not use it if it still has more to go

@gupnik gupnik added this pull request to the merge queue May 29, 2024
Merged via the queue into master with commit 5f68c93 May 29, 2024
156 of 157 checks passed
@gupnik gupnik deleted the gupnik/runtime-macro branch May 29, 2024 04:06
ordian added a commit that referenced this pull request May 30, 2024
* master: (93 commits)
  Fix broken windows build (#4636)
  Beefy client generic on aduthority Id (#1816)
  pallet-staking: Put tests behind `cfg(debug_assertions)` (#4620)
  Broker new price adapter (#4521)
  Change `XcmDryRunApi::dry_run_extrinsic` to take a call instead (#4621)
  Update README.md (#4623)
  Publish `chain-spec-builder` (#4518)
  Add omni bencher & chain-spec-builder bins to release (#4557)
  Moves runtime macro out of experimental flag (#4249)
  Filter workspace dependencies in the templates (#4599)
  parachain-inherent: Make `para_id` more prominent (#4555)
  Add metric to measure the time it takes to gather enough assignments (#4587)
  Improve On_demand_assigner events (#4339)
  Conditional `required` checks (#4544)
  [CI] Deny adding git deps (#4572)
  [subsytem-bench] Remove redundant banchmark_name param (#4540)
  Add availability-recovery from systematic chunks (#1644)
  Remove workspace lints from templates (#4598)
  `sc-chain-spec`: deprecated code removed (#4410)
  [subsystem-benchmarks] Add statement-distribution benchmarks (#3863)
  ...
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this pull request Jun 5, 2024
Step in paritytech#3688

Now that the `runtime` macro (Construct Runtime V2) has been
successfully deployed on Westend, this PR moves it out of the
experimental feature flag and makes it generally available for runtime
devs.

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
Step in paritytech#3688

Now that the `runtime` macro (Construct Runtime V2) has been
successfully deployed on Westend, this PR moves it out of the
experimental feature flag and makes it generally available for runtime
devs.

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants