-
Notifications
You must be signed in to change notification settings - Fork 828
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
effort towards getting chainspecbuilder into omni-node fix 5567 #7619
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a polkadot-omni-node
README we can update so that when referencing building a chain spec, we specify the chain-spec-builder
subcommand.
@@ -24,6 +24,7 @@ futures = { workspace = true } | |||
log = { workspace = true, default-features = true } | |||
serde = { features = ["derive"], workspace = true, default-features = true } | |||
serde_json = { workspace = true, default-features = true } | |||
chain-spec-builder = { workspace = true , package = "staging-chain-spec-builder" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to mention the package? Thought chain-spec-builder = { workspace = true }
should do it.
@@ -56,7 +56,7 @@ pub enum ChainSpecBuilderCmd { | |||
} | |||
|
|||
/// Create a new chain spec by interacting with the provided runtime wasm blob. | |||
#[derive(Parser, Debug)] | |||
#[derive(Parser, Clone, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: same comment as for ChainSpecBuilder
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree I can remove this one, I added it as precautionary
I will add the prdoc and resolve conflicts now, for reference tagging @iulianbarbu @skunert |
After suggested changes, updated command @iulianbarbu |
All GitHub workflows were cancelled due to failure one of the required jobs. |
This reverts commit d7fb6a6.
if let Err(err) = cmd.run() { | ||
eprintln!("Error executing chain-spec-builder: {}", err); | ||
return Err(sc_cli::Error::Application(err.into())); | ||
} | ||
|
||
Ok(()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this won't look better?
if let Err(err) = cmd.run() { | |
eprintln!("Error executing chain-spec-builder: {}", err); | |
return Err(sc_cli::Error::Application(err.into())); | |
} | |
Ok(()) | |
cmd.run() | |
.map_err(|err| { | |
eprintln!("Error executing chain-spec-builder: {}", err) | |
Error::Application(err.into()) | |
) |
Adding chain-spec-builder as a subcommand into Polkadot omni node