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

Migrate pallet-mmr to umbrella crate #7081

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

yrong
Copy link
Contributor

@yrong yrong commented Jan 8, 2025

Part of #6504

@yrong yrong marked this pull request as ready for review January 8, 2025 08:28
@yrong yrong requested review from acatangiu and a team as code owners January 8, 2025 08:28
@acatangiu acatangiu added R0-silent Changes should not be mentioned in any release notes T2-pallets This PR/Issue is related to a particular pallet. labels Jan 8, 2025
.gitignore Outdated Show resolved Hide resolved
frame-benchmarking = { optional = true, workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
frame = { workspace = true, features = ["experimental", "runtime"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the experimental feature for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just follow other PRs. Yeah, it's a bit confusing, it won't compile after removing the feature.

polkadot-sdk git:(migrate-pallet-mmr) ✗ cargo check -p pallet-mmr
...
error[E0433]: failed to resolve: could not find `deps` in `frame`
  --> substrate/frame/merkle-mountain-range/src/lib.rs:65:2
   |
65 |     deps::sp_mmr_primitives::{
   |     ^^^^ could not find `deps` in `frame`
...
68 |     prelude::*,
   |     ^^^^^^^ could not find `prelude` in `frame`
...

Same concern in #6504 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove the experimental feature gate in frame crate:

diff --git a/substrate/frame/src/lib.rs b/substrate/frame/src/lib.rs
index f79a52bc6c5..44907a877d7 100644
--- a/substrate/frame/src/lib.rs
+++ b/substrate/frame/src/lib.rs
@@ -150,7 +150,6 @@
 //! * `runtime::apis` should expose all common runtime APIs that all FRAME-based runtimes need.
 
 #![cfg_attr(not(feature = "std"), no_std)]
-#![cfg(feature = "experimental")]
 
 #[doc(no_inline)]
 pub use frame_support::pallet;

Those PR is like making all pallets experimental.

I'll will put this message on the main issue tracker

@yrong yrong changed the title Migrate pallet mmr to umbrella crate Migrate pallet-mmr to umbrella crate Jan 8, 2025
substrate/frame/merkle-mountain-range/src/tests.rs Outdated Show resolved Hide resolved
substrate/frame/merkle-mountain-range/src/tests.rs Outdated Show resolved Hide resolved
substrate/frame/merkle-mountain-range/src/tests.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

Looks good, but we should rather not use the feature experimental, I open discussion on the main issue tracker

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T2-pallets This PR/Issue is related to a particular pallet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants