-
Notifications
You must be signed in to change notification settings - Fork 689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged network-primitives into network package #7722
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To give more context here: the x
/ x-primitives
split historically goes along our public RPC boundary. But there's just a couple of types which atually are on the RPC bounday, and we just moved them over to client-primitives.
type From = near_network::types::AccountOrPeerIdOrHash; | ||
type To = near_client_primitives::types::AccountOrPeerIdOrHash; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice pattern, going to add it to my toolbox!
//! WARNING WARNING WARNING | ||
//! We need to maintain backwards compatibility, all changes to this file needs to be reviews. | ||
use crate::network_protocol::edge::{Edge, PartialEdgeInfo}; | ||
use crate::network_protocol::{PeerChainInfoV2, PeerInfo, RoutedMessage}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we kill borsh already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried recently, but our python test suite is very messy and it would take me a few weeks to remove borsh usage there. I don't have that capacity now.
Part of the public API of network package was placed in near-network-primitives and part was in near-network. Hence most of the other packages imported both anyway. Merging them back together allows us to: - decrease the public API surface (since within a crate visibility settings are more flexible) - put API next to the only implementation of that API - decrease the confusion when importing network stuff in other packages
Part of the public API of network package was placed in near-network-primitives and part was in near-network. Hence most of the other packages imported both anyway. Merging them back together allows us to: