Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Companion PR for Add a build-sync-spec subcommand and remove the CHT roots from the light sync state. #1670

Merged
6 commits merged into from
Sep 11, 2020

Conversation

expenses
Copy link
Contributor

@expenses expenses commented Sep 1, 2020

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.

@expenses expenses added A0-please_review Pull request needs code review. A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Sep 1, 2020
@expenses
Copy link
Contributor Author

@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))
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor

@seunlanlege seunlanlege left a comment

Choose a reason for hiding this comment

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

looks solid

Copy link
Contributor

@cecton cecton left a 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

@expenses
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Sep 11, 2020

Waiting for commit status.

@ghost
Copy link

ghost commented Sep 11, 2020

Checks failed; merge aborted.

@expenses
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Sep 11, 2020

Waiting for commit status.

@ghost
Copy link

ghost commented Sep 11, 2020

Checks failed; merge aborted.

@expenses
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Sep 11, 2020

Waiting for commit status.

@ghost ghost merged commit 6968004 into master Sep 11, 2020
@ghost ghost deleted the ashley-build-sync-spec branch September 11, 2020 15:08
ordian added a commit that referenced this pull request Sep 14, 2020
* 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)
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A4-companion A PR that needs a companion PR to merge in parallel for one of its downstream dependencies. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants