peer selection exports #5081
peer selection exports #5081
Conversation
coot
force-pushed
the
coot/peer-selection-exports
branch
from
bb7e605 to
0255364
Compare
Just to make it similar to test libraries in other packages.
coot
force-pushed
the
coot/peer-selection-exports
branch
from
0255364 to
e233433
Compare
There was a problem hiding this comment.
I am not entirely sold on this import changes. It is nice to clean up imports and simplify them but I think meta modules are a bit of a stretch. I usually import the module that owns the expression definition or data type, I don't usually import from top level modules like these, I think it makes it clearer where things are coming from.
| {-# LANGUAGE ExplicitNamespaces #-} | ||
|
|
||
| module Ouroboros.Network.PeerSelection | ||
| ( module Governor | ||
| , module PeerSelection | ||
| , module PeerSelection.Types | ||
| , module PeerSelection.PublicRootPeers | ||
| , module PeerSelection.PeerStateActions | ||
| , module PeerSelection.PeerSelectionActions | ||
| , module PeerSelection.RelayAccessPoint | ||
| , module PeerSelection.LedgerPeers | ||
| , module PeerSelection.PeerMetrics | ||
| , module PeerSelection.Churn | ||
| , module PeerSelection.PeerAdvertise | ||
| , module PeerSelection.PeerSharing | ||
| ) where | ||
|
|
||
| import Ouroboros.Network.PeerSelection.Churn as PeerSelection.Churn | ||
| -- Only essential `Governor` types. | ||
| import Ouroboros.Network.PeerSelection.Governor as Governor | ||
| (DebugPeerSelection (..), PeerSelectionActions, | ||
| PeerSelectionInterfaces (..), PeerSelectionPolicy (..), | ||
| PeerSelectionState, PeerSelectionTargets (..), PeerStateActions, | ||
| PickPolicy) | ||
| import Ouroboros.Network.PeerSelection.LedgerPeers as PeerSelection.LedgerPeers | ||
| (AfterSlot (..), IsBigLedgerPeer (..), LedgerPeerSnapshot (..), | ||
| LedgerPeers (..), LedgerPeersConsensusInterface (..), | ||
| LedgerPeersKind (..), NumberOfPeers (..), TraceLedgerPeers (..), | ||
| UseLedgerPeers (..), WithLedgerPeersArgs (..), withLedgerPeers) | ||
| import Ouroboros.Network.PeerSelection.PeerAdvertise as PeerSelection.PeerAdvertise | ||
| import Ouroboros.Network.PeerSelection.PeerMetric as PeerSelection.PeerMetrics | ||
| (PeerMetrics, PeerMetricsConfiguration (..), ReportPeerMetrics (..), | ||
| newPeerMetric, newPeerMetric', nullMetric, reportMetric) | ||
| import Ouroboros.Network.PeerSelection.PeerSelectionActions as PeerSelection.PeerSelectionActions | ||
| import Ouroboros.Network.PeerSelection.PeerSharing as PeerSelection.PeerSharing | ||
| import Ouroboros.Network.PeerSelection.PeerStateActions as PeerSelection.PeerStateActions | ||
| import Ouroboros.Network.PeerSelection.PublicRootPeers as PeerSelection.PublicRootPeers | ||
| (PublicRootPeers) | ||
| import Ouroboros.Network.PeerSelection.RelayAccessPoint as PeerSelection.RelayAccessPoint | ||
| (DomainAccessPoint (..), IP (..), PortNumber, RelayAccessPoint (..)) | ||
| import Ouroboros.Network.PeerSelection.Types as PeerSelection | ||
| import Ouroboros.Network.PeerSelection.Types as PeerSelection.Types |
There was a problem hiding this comment.
I don't like this... I prefer for the imports to come from the place where the expression is defined. Meta modules like these are confusing for large codebases
There was a problem hiding this comment.
It's annoying when we have to solve 10s of rebase conflicts in imports; additionally we often introduced more mess into import statements which makes it less readable.
Finding out where something comes from is not that difficult, if one knows that it's coming from PeerSelection, or RootPeersDNS even without tools like hls or ghc-tags-plugin, a simple grep is enough (although once one is familiar it's easy to guess).
There was a problem hiding this comment.
Another solution could be to have explicit imports in the meta modules, but I am not sure if it's worth it in times of hls / ghc-tags-plugin etc.
There was a problem hiding this comment.
Right but, I also use HLS to manage automated imports for me and I usually import from the modules which defined the expression/function. So my worry is that we'll have inconsistencies still in the imports. At least I think the name should be more obvious so I know I should import from the Meta/General/Imports/All modules
| module Ouroboros.Network.PeerSelection.RootPeersDNS | ||
| ( PeerActionsDNS (..) | ||
| , module DNSActions | ||
| , module DNSSemaphore | ||
| , module LedgerPeers | ||
| , module LocalRootPeers | ||
| , module PublicRootPeers | ||
| ) where | ||
|
|
||
| import Data.IP (IP) | ||
| import Network.Socket (PortNumber) | ||
|
|
||
| import Ouroboros.Network.PeerSelection.RootPeersDNS.DNSActions | ||
|
|
||
| -- | Record of some parameters that are commonly used together | ||
| -- | ||
| data PeerActionsDNS peeraddr resolver exception m = PeerActionsDNS { | ||
| paToPeerAddr :: IP -> PortNumber -> peeraddr, | ||
| paDnsActions :: DNSActions resolver exception m | ||
| } | ||
| import Ouroboros.Network.PeerSelection.RootPeersDNS.DNSActions as DNSActions | ||
| import Ouroboros.Network.PeerSelection.RootPeersDNS.DNSSemaphore as DNSSemaphore | ||
| import Ouroboros.Network.PeerSelection.RootPeersDNS.LedgerPeers as LedgerPeers | ||
| import Ouroboros.Network.PeerSelection.RootPeersDNS.LocalRootPeers as LocalRootPeers | ||
| import Ouroboros.Network.PeerSelection.RootPeersDNS.PublicRootPeers as PublicRootPeers |
There was a problem hiding this comment.
Same here. We can move PeerActionsDNS to a better module but we should import it from there.
Depends on #5007