-
Notifications
You must be signed in to change notification settings - Fork 214
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
Fix clippy warnings #500
base: master
Are you sure you want to change the base?
Fix clippy warnings #500
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,20 +84,20 @@ mod test { | |
("20 kB", 20 * 1000), | ||
("20K", 20 * 1000), | ||
(" 20k", 20 * 1000), | ||
("1MB", 1 * 1000 * 1000), | ||
("1M", 1 * 1000 * 1000), | ||
("1m", 1 * 1000 * 1000), | ||
("1 m", 1 * 1000 * 1000), | ||
("1GB", 1 * 1000 * 1000 * 1000), | ||
("1G", 1 * 1000 * 1000 * 1000), | ||
("1g", 1 * 1000 * 1000 * 1000), | ||
("1KiB", 1 * 1024), | ||
("1Ki", 1 * 1024), | ||
("1MiB", 1 * 1024 * 1024), | ||
("1Mi", 1 * 1024 * 1024), | ||
("1GiB", 1 * 1024 * 1024 * 1024), | ||
("1Gi", 1 * 1024 * 1024 * 1024), | ||
(" 1 Gi ", 1 * 1024 * 1024 * 1024), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these are just silly, so much more readable before :) perhaps we could should add a clippy config for style lints like this one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be up for a clippy config being added to telemetry so that we can keep more of what we like; I'm generally happy with these changes but there isn't much value in adding Default impls and a couple of other nits I have (but I htink nits are allowed since that's what clippy is all about!) |
||
("1MB", 1000 * 1000), | ||
("1M", 1000 * 1000), | ||
("1m", 1000 * 1000), | ||
("1 m", 1000 * 1000), | ||
("1GB", 1000 * 1000 * 1000), | ||
("1G", 1000 * 1000 * 1000), | ||
("1g", 1000 * 1000 * 1000), | ||
("1KiB", 1024), | ||
("1Ki", 1024), | ||
("1MiB", 1024 * 1024), | ||
("1Mi", 1024 * 1024), | ||
("1GiB", 1024 * 1024 * 1024), | ||
("1Gi", 1024 * 1024 * 1024), | ||
(" 1 Gi ", 1024 * 1024 * 1024), | ||
]; | ||
|
||
for (s, expected) in cases { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,13 +55,13 @@ pub enum FromShardWebsocket { | |
Add { | ||
local_id: ShardNodeId, | ||
ip: std::net::IpAddr, | ||
node: common::node_types::NodeDetails, | ||
node: Box<common::node_types::NodeDetails>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think that is small enough to avoid to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, but enum variant is around 369 bytes:
|
||
genesis_hash: common::node_types::BlockHash, | ||
}, | ||
/// Update/pass through details about a node. | ||
Update { | ||
local_id: ShardNodeId, | ||
payload: node_message::Payload, | ||
payload: Box<node_message::Payload>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Clippy seems to use 200 as default, https://rust-lang.github.io/rust-clippy/master/#large_enum_variant |
||
}, | ||
/// Tell the aggregator that a node has been removed when it disconnects. | ||
Remove { local_id: ShardNodeId }, | ||
|
@@ -139,7 +139,7 @@ impl FromStr for FromFeedWebsocket { | |
"subscribe" => Ok(FromFeedWebsocket::Subscribe { | ||
chain: value.parse()?, | ||
}), | ||
_ => return Err(anyhow::anyhow!("Command {} not recognised", cmd)), | ||
_ => Err(anyhow::anyhow!("Command {} not recognised", cmd)), | ||
} | ||
} | ||
} | ||
|
@@ -235,16 +235,16 @@ impl InnerLoop { | |
// ignore node updates if we have too many messages to handle, in an attempt | ||
// to reduce the queue length back to something reasonable, lest it get out of | ||
// control and start consuming a load of memory. | ||
if metered_tx.len() > max_queue_len { | ||
if matches!( | ||
if metered_tx.len() > max_queue_len | ||
&& matches!( | ||
msg, | ||
ToAggregator::FromShardWebsocket(.., FromShardWebsocket::Update { .. }) | ||
) { | ||
// Note: this wraps on overflow (which is probably the best | ||
// behaviour for graphing it anyway) | ||
dropped_messages.fetch_add(1, Ordering::Relaxed); | ||
continue; | ||
} | ||
) | ||
{ | ||
// Note: this wraps on overflow (which is probably the best | ||
// behaviour for graphing it anyway) | ||
dropped_messages.fetch_add(1, Ordering::Relaxed); | ||
continue; | ||
} | ||
|
||
if let Err(e) = metered_tx.send(msg) { | ||
|
@@ -327,7 +327,7 @@ impl InnerLoop { | |
} => { | ||
// Conditionally modify the node's details to include the IP address. | ||
node.ip = self.expose_node_ips.then_some(ip.to_string().into()); | ||
match self.node_state.add_node(genesis_hash, node) { | ||
match self.node_state.add_node(genesis_hash, *node) { | ||
state::AddNodeResult::ChainOnDenyList => { | ||
if let Some(shard_conn) = self.shard_channels.get_mut(&shard_conn_id) { | ||
let _ = shard_conn.send(ToShardWebsocket::Mute { | ||
|
@@ -359,7 +359,7 @@ impl InnerLoop { | |
let mut feed_messages_for_chain = FeedMessageSerializer::new(); | ||
feed_messages_for_chain.push(feed_message::AddedNode( | ||
node_id.get_chain_node_id().into(), | ||
&details.node, | ||
details.node, | ||
)); | ||
self.finalize_and_broadcast_to_chain_feeds( | ||
&genesis_hash, | ||
|
@@ -411,7 +411,7 @@ impl InnerLoop { | |
|
||
let mut feed_message_serializer = FeedMessageSerializer::new(); | ||
self.node_state | ||
.update_node(node_id, payload, &mut feed_message_serializer); | ||
.update_node(node_id, *payload, &mut feed_message_serializer); | ||
|
||
if let Some(chain) = self.node_state.get_chain_by_node_id(node_id) { | ||
let genesis_hash = chain.genesis_hash(); | ||
|
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 guess
#[derive(Default)]
didn't work? I'd prefer that if possible :)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.
BiMap
doesn't impl Default