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

Improvements in solochain template #5111

Closed
wants to merge 20 commits into from
Closed

Improvements in solochain template #5111

wants to merge 20 commits into from

Conversation

gupnik
Copy link
Contributor

@gupnik gupnik commented Jul 23, 2024

This PR adds the following changes:


  • Exposes sp-genesis-builder via the frame crate
  • Exposes serde feature in the frame crate
  • Exposes public types via an interface in the solochain runtime

@gupnik gupnik added the R0-silent Changes should not be mentioned in any release notes label Jul 23, 2024
@gupnik gupnik requested a review from a team as a code owner July 23, 2024 04:49
templates/solochain/runtime/src/lib.rs Outdated Show resolved Hide resolved
templates/solochain/runtime/src/lib.rs Outdated Show resolved Hide resolved
@@ -233,6 +233,7 @@ pub mod runtime {
pub use sp_block_builder::*;
pub use sp_consensus_aura::*;
pub use sp_consensus_grandpa::*;
pub use sp_genesis_builder::*;
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should directly pull in the Result here as GenesisBuilderResult. Maybe directly rename it in the crate to make it even more obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO we should directly pull in the Result here as GenesisBuilderResult. Maybe directly rename it in the crate to make it even more obvious.

Exported it from the frame crate but didn't rename in sp-genesis-builder to avoid a breaking change.

@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 24, 2024 05:46
templates/solochain/runtime/Cargo.toml Outdated Show resolved Hide resolved
@bkchr bkchr requested a review from kianenigma July 24, 2024 14:34
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 3/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6801096

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable 2/3
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6801095

ConstBool, ConstU128, ConstU32, ConstU64, ConstU8, KeyOwnerProofSystem, Randomness,
StorageInfo,
use frame::{
deps::{
Copy link
Contributor

Choose a reason for hiding this comment

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

#5155 (comment) #5237

I mean, if we use deps, there is little benefit in using this crate overall, other than a cleaner Cargo.toml file.

I am tempted to even add a warning to deps to prevent deps being used recklessly..

vec![]
}
}
}

pub mod interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good idea, isolating what needs to be exported to node or other crates.

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.

@kianenigma kianenigma added the T17-Templates This PR/Issue is related to templates label Aug 5, 2024
@ggwpez
Copy link
Member

ggwpez commented Sep 6, 2024

The T17 label was already allocated here: paritytech/labels#42
please remove

@kianenigma
Copy link
Contributor

Will this be completed? we can find someone else to finish it perhaps.

@HCastano
Copy link
Contributor

@kianenigma @gupnik FWIW I'd find this pretty useful if it got merged 🙏

@kianenigma
Copy link
Contributor

@kianenigma @gupnik FWIW I'd find this pretty useful if it got merged 🙏

@HCastano would you be down to finish it? I can mentor and re-evaluate :)

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 T17-Templates This PR/Issue is related to templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants