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

Revisit location or existence of NetworkStatus #7032

Closed
romanb opened this issue Sep 6, 2020 · 2 comments · Fixed by #8748
Closed

Revisit location or existence of NetworkStatus #7032

romanb opened this issue Sep 6, 2020 · 2 comments · Fixed by #8748
Assignees
Labels
I4-annoyance The client behaves within expectations, however this “expected behaviour” itself is at issue. I7-refactor Code needs refactoring. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder

Comments

@romanb
Copy link
Contributor

romanb commented Sep 6, 2020

by @tomaka from #6986 (comment):

NetworkStatus was moved by someone not in the networking team as part of a big PR, and I didn't spot the change.

The entire point of NetworkStatus is to communicate some networking-related information internally between some components without having to depend on sc-network.
Moving it to sc-network entirely defeats this purpose.

It should either be moved back for example to sc-service, or be removed entirely and have everything depend on Arc<NetworkService>.

@romanb romanb added I4-annoyance The client behaves within expectations, however this “expected behaviour” itself is at issue. I7-refactor Code needs refactoring. labels Sep 6, 2020
@tomaka tomaka added the Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder label May 3, 2021
@kpp kpp self-assigned this May 4, 2021
@kpp
Copy link
Contributor

kpp commented May 4, 2021

A little bit of context for this issue:

@kpp
Copy link
Contributor

kpp commented May 4, 2021

Dependent crates of struct NetworkStatus:

  • sc-informant
  • sc-network
  • sc-service

Cargo trees:

  • sc-informant -> sc-network
  • sc-service -> (other deps...) -> sc-network

So no wonder the struct is located in sc-network. Placing the struct inside sc-service create cyclic package dependency...

@ghost ghost closed this as completed in #8748 May 27, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I4-annoyance The client behaves within expectations, however this “expected behaviour” itself is at issue. I7-refactor Code needs refactoring. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants