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

default ws note #10014

Closed
wants to merge 12 commits into from
Closed

default ws note #10014

Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
85dda1d
defalut ws note
nuke-web3 Oct 12, 2021
28fa065
Update client/cli/src/commands/run_cmd.rs
nuke-web3 Oct 12, 2021
7daf30d
Update client/cli/src/commands/run_cmd.rs
nuke-web3 Oct 12, 2021
da1cdd0
Use CLI default for ws-max-connections
adoerr Oct 14, 2021
77b8e4a
cargo fmt
adoerr Oct 14, 2021
2aabcb3
Merge branch 'master' into NukeManDan-patch-2
adoerr Oct 14, 2021
7b6d2ff
No usize::MAX
adoerr Oct 14, 2021
569f634
Update bin/node/cli/benches/transaction_pool.rs
adoerr Oct 14, 2021
9946d6a
Update client/cli/src/config.rs
adoerr Oct 14, 2021
82d56a9
Update client/service/test/src/lib.rs
adoerr Oct 14, 2021
163641b
Update client/cli/src/commands/run_cmd.rs
adoerr Oct 14, 2021
c096841
Update test-utils/test-runner/src/utils.rs
adoerr Oct 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions client/cli/src/commands/run_cmd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ pub struct RunCmd {
pub ws_port: Option<u16>,

/// Maximum number of WS RPC server connections.
///
/// Default is 100.
// {this is set in client/rpc-servers/src/lib.rs}
#[structopt(long = "ws-max-connections", value_name = "COUNT")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can;t we do something like

#[structopt(long = "ws-max-connections", value_name = "COUNT", default_value="100")]?

And get rid of sc_rpc_server::WS_MAX_CONNECTION?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would also avoid the (likely) miss of the comment needing to be changed if this constant ever changed. grouped in the same lines here would be much easier to spot 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, using default_value doesn't work with using an Option<>. So this change would have quite a bit of ripple effect.

For now, I gues, the updated comment will have to do.

Copy link
Member

Choose a reason for hiding this comment

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

What ripple effect?

pub ws_max_connections: Option<usize>,

Expand Down