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

Incorporate sc-peerset into sc-network#14236

Merged
dmitry-markin merged 4 commits intomasterfrom
dm-move-peerstore-to-network
May 29, 2023
Merged

Incorporate sc-peerset into sc-network#14236
dmitry-markin merged 4 commits intomasterfrom
dm-move-peerstore-to-network

Conversation

@dmitry-markin
Copy link
Contributor

@dmitry-markin dmitry-markin commented May 26, 2023

This is a preparation PR for #14170. Nothing interesting is happening here, just Peerset, PeerStore, and ProtocolController are moved from sc-peerset to sc-network & sc-network-common.

Adding B1-note_worthy label because the sc-peerset crate is retired.

@dmitry-markin dmitry-markin added A0-please_review Pull request needs code review. 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 B1-note_worthy Changes should be noted in the release notes labels May 26, 2023
@dmitry-markin dmitry-markin requested a review from a team May 26, 2023 11:56
@dmitry-markin dmitry-markin added the T0-node This PR/Issue is related to the topic “node”. label May 26, 2023
altonen
altonen approved these changes May 26, 2023
View reviewed changes
@dmitry-markin dmitry-markin requested a review from a team May 26, 2023 13:15
melekes
melekes approved these changes May 28, 2023
View reviewed changes
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@dmitry-markin
Copy link
Contributor Author

dmitry-markin commented May 29, 2023

IDK why, but after seemingly trivial changes the nodes in reconnect_after_disconnect Notifications test stopped receiving any network events. Investigating.

@altonen
Copy link
Contributor

altonen commented May 29, 2023

Maybe it was flakey/made flakey by some recent sc-network change and just got triggered in this PR?

@dmitry-markin
Copy link
Contributor Author

dmitry-markin commented May 29, 2023

Maybe it was flakey/made flakey by some recent sc-network change and just got triggered in this PR?

It doesn't look like flaky, because it always succeeds in master, and always hangs in this PR.

altonen
altonen reviewed May 29, 2023
View reviewed changes
@dmitry-markin dmitry-markin merged commit a8e7ba1 into master May 29, 2023
@dmitry-markin dmitry-markin deleted the dm-move-peerstore-to-network branch May 29, 2023 17:56
dmitry-markin added a commit that referenced this pull request Jul 3, 2023
@dmitry-markin
Revert "Incorporate sc-peerset into sc-network (#14236)"
bbee969 
This reverts commit a8e7ba1.
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@dmitry-markin @nathanwhit
Incorporate sc-peerset into sc-network (paritytech#14236)
5da3f71
@altonen altonen mentioned this pull request Jul 20, 2023
Open
@crystalin crystalin mentioned this pull request Oct 20, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@altonen altonen altonen approved these changes

@melekes melekes melekes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the 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 T0-node This PR/Issue is related to the topic “node”.

Projects

Networking
Status: Done ✅

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dmitry-markin @altonen @melekes