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

Network sync refactoring (part 5) #11825

Conversation

nazar-pc
Copy link
Contributor

Another step towards #8686

This addresses ugliness in configuration with factory method after #11412 as discussed in #11412 (comment)

Not entirely happy with sync mode matching in multiple places, open for suggestions.

@nazar-pc nazar-pc force-pushed the network-sync-refactoring-part-5 branch from 3bacc37 to 18cde57 Compare July 24, 2022 03:46
@nazar-pc
Copy link
Contributor Author

Resolved merge conflicts after #11865.

@bkchr PTAL.
I have much bigger refactoring pending submission and due to the nature of changes long delays make it harder to resolve merge conflicts.

@nazar-pc
Copy link
Contributor Author

@arkpar you were reviewing previous PR, maybe you can take a look?

@arkpar
Copy link
Member

arkpar commented Jul 29, 2022

I'd suggest removing sc_network::config::SyncMode and simply using sc_network_common::sync::SyncMode everywhere.
Looks good otherwise.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

As always, sorry for the delay!

@bkchr bkchr added A0-please_review Pull request needs code review. 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jul 29, 2022
@bkchr
Copy link
Member

bkchr commented Jul 29, 2022

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit 8f4d1b3 into paritytech:master Jul 29, 2022
@nazar-pc nazar-pc deleted the network-sync-refactoring-part-5 branch July 29, 2022 20:17
DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
* Make `chain_sync` an explicit networking parameter instead of offering factory method

* Derive `Copy` on `SyncMode` and remove cloning
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Make `chain_sync` an explicit networking parameter instead of offering factory method

* Derive `Copy` on `SyncMode` and remove cloning
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. 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants