-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Companion PR for Add a build-sync-spec subcommand and remove the CHT roots from the light sync state.
#1670
Conversation
|
@cecton do you think you could quickly review this please? Thanks! |
| )?; | ||
| let client = Arc::new(client); | ||
|
|
||
| Ok((cmd.run(chain_spec, network_config, client, network_status_sinks), task_manager)) |
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.
why not just pass the Configuration struct? chain_spec and network_config live on the Configuration object
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.
The Configuration gets consumed by new_full_nongeneric, so it can't be used.
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 would argue that it should be clone, but that's for another time. This looks solid to me
seunlanlege
left a comment
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.
looks solid
cecton
left a comment
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.
Seems good to me
|
bot merge |
|
Waiting for commit status. |
|
Checks failed; merge aborted. |
|
bot merge |
|
Waiting for commit status. |
|
Checks failed; merge aborted. |
|
bot merge |
|
Waiting for commit status. |
* master: Companion PR for #6984 (#1661) Update some dependencies. (#1718) Add a specific memory requirements (#1716) Companion PR for #7039: grandpa-rpc dont share subscription manager, only executor (#1687) Update bytes. (#1715) Add a note about memory requirements (#1714) Update parity-multiaddr. (#1700) typo in proxy tests (#1713) Companion PR for ` Add a `build-sync-spec` subcommand and remove the CHT roots from the light sync state.` (#1670) Forwardport: Validation: don't detect STDIN closing when running in process (#1695) (#1703)
See paritytech/substrate#6999. Not strictly a companion PR, as the substrate PR doesn't break anything. It would still be nice to get them merged at the same time though.