Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Define ClusterConfig::new_with_equal_stakes() #32330

Merged
merged 2 commits into from
Jun 30, 2023

Conversation

ryoqun
Copy link
Contributor

@ryoqun ryoqun commented Jun 29, 2023

Problem

another small refactor for #31239 per #31239 (comment)

Summary of Changes

Fixes #

@codecov
Copy link

codecov bot commented Jun 29, 2023

Codecov Report

Merging #32330 (0d5e602) into master (5b6c3ba) will increase coverage by 0.0%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #32330   +/-   ##
=======================================
  Coverage    82.0%    82.1%           
=======================================
  Files         773      773           
  Lines      209748   209748           
=======================================
+ Hits       172180   172216   +36     
+ Misses      37568    37532   -36     

Copy link
Contributor

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

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

couple nits

@@ -93,6 +93,25 @@ pub struct ClusterConfig {
pub tpu_connection_pool_size: usize,
}

impl ClusterConfig {
pub fn new_with_equal_stakes(
Copy link
Contributor

Choose a reason for hiding this comment

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

does / should this be pub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems so... even pub(crate) didn't work (tested at #31239):

error[E0624]: associated function `new_with_equal_stakes` is private
    --> local-cluster/tests/local_cluster.rs:3038:37
     |
3038 |       let mut config = ClusterConfig::new_with_equal_stakes(
     |                                       ^^^^^^^^^^^^^^^^^^^^^ private associated function
     |
    ::: /home/ryoqun/work/solana/for-merge/local-cluster/src/local_cluster.rs:96:5
     |
96   | /     pub(crate) fn new_with_equal_stakes(
97   | |         num_nodes: usize,
98   | |         cluster_lamports: u64,
99   | |         lamports_per_node: u64,
100  | |     ) -> Self {
     | |_____________- private associated function defined here

For more information about this error, try `rustc --explain E0624`.
error: could not compile `solana-local-cluster` (test "local_cluster") due to previous error
warning: build failed, waiting for other jobs to finish...

Comment on lines 102 to 104
let stakes: Vec<_> = vec![lamports_per_node; num_nodes];
Self {
node_stakes: stakes,
Copy link
Contributor

Choose a reason for hiding this comment

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

realize it was just moved, but let's do a tactical rename in this PR:

Suggested change
let stakes: Vec<_> = vec![lamports_per_node; num_nodes];
Self {
node_stakes: stakes,
let node_stakes = vec![lamports_per_node; num_nodes];
Self {
node_stakes,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how about this? 0d5e602 better, imo. :)

@ryoqun ryoqun requested a review from apfitzge June 30, 2023 00:23
Copy link
Contributor

@apfitzge apfitzge left a comment

Choose a reason for hiding this comment

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

Lgtm

@ryoqun ryoqun merged commit f949a95 into solana-labs:master Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants