Accept ip_port and net_id as Option<String> or Option<&str> for sbot::init() #52

glyph · 2022-07-13T10:22:17Z

glyph commented

2022-07-13 10:22:17 +00:00

Fairly simple change that gives us more flexibility when calling sbot::init() by allowing the ip_port and net_id parameters to be passed as Option<String> or Option<&str>.

This makes it possible to define const values for the parameters required by sbot::init(), for example:

const KEYSTORE: Keystore = Keystore::GoSbot;
const IP_PORT: Option<&str> = Some("127.0.0.1:8021");
const NET_ID: Option<&str> = None;

Then any sbot initialisation can reuse those values:

let mut sbot = Sbot::init(KEYSTORE, IP_PORT, NET_ID)
    .await
    .map_err(|e| e.to_string())?;

The downside is that None parameters need to be annotated when the function is called. For example:

Sbot::init(Keystore::CustomGoSbot(key_path), None::<&str>, None::<&str>).await?

Notice the None::<&str> annotation.

@notplants What do you think? I'm happy to make the required changes to peach-web and peach-config, along with the various documentation, but wanted to check-in with you because this represents a breaking change.

This was prompted by my work on lykin, specifically wanting to be able to quickly define a custom port for go-sbot.

Fairly simple change that gives us more flexibility when calling `sbot::init()` by allowing the `ip_port` and `net_id` parameters to be passed as `Option<String>` or `Option<&str>`. This makes it possible to define `const` values for the parameters required by `sbot::init()`, for example: ```rust const KEYSTORE: Keystore = Keystore::GoSbot; const IP_PORT: Option<&str> = Some("127.0.0.1:8021"); const NET_ID: Option<&str> = None; ``` Then any `sbot` initialisation can reuse those values: ```rust let mut sbot = Sbot::init(KEYSTORE, IP_PORT, NET_ID) .await .map_err(|e| e.to_string())?; ``` The downside is that `None` parameters need to be annotated when the function is called. For example: ```rust Sbot::init(Keystore::CustomGoSbot(key_path), None::<&str>, None::<&str>).await? ``` Notice the `None::<&str>` annotation. @notplants What do you think? I'm happy to make the required changes to `peach-web` and `peach-config`, along with the various documentation, but wanted to check-in with you because this represents a breaking change. ----- This was prompted by my work on `lykin`, specifically wanting to be able to quickly define a custom port for go-sbot.

glyph added 1 commit 2022-07-13 10:22:18 +00:00

2f347ee0d6 accept String or &str for ip_port and net_id

glyph requested review from notplants 2022-07-19 07:42:27 +00:00

notplants commented

2022-07-19 09:28:19 +00:00

Interesting, it seems like a natural thing to want to be able to define, but that None::<&str> syntax is pretty bizarre. But it seems like thats the way to do "optional" arguments in rust so looks good.

Then any sbot initialisation can reuse those values:

fwiw, the pattern I would probably use in the application code is to define a separate utility function with no arguments init_sbot(), which calls Sbot::init(KEYSTORE, IP_PORT, NET_ID) inside of it, rather than calling Sbot::init in multiple places passing the same constant arguments (and thus avoiding the code duplication of needing the same arguments to be passed)

Interesting, it seems like a natural thing to want to be able to define, but that None::<&str> syntax is pretty bizarre. But it seems like thats the way to do "optional" arguments in rust so looks good. > Then any sbot initialisation can reuse those values: fwiw, the pattern I would probably use in the application code is to define a separate utility function with no arguments init_sbot(), which calls Sbot::init(KEYSTORE, IP_PORT, NET_ID) inside of it, rather than calling Sbot::init in multiple places passing the same constant arguments (and thus avoiding the code duplication of needing the same arguments to be passed)

👍 1

glyph commented

2022-07-19 09:40:05 +00:00

Haha yeah the turbofish is definitely one of the weirder pieces of Rust syntax.

the pattern I would probably use in the application code is to define a separate utility function with no arguments init_sbot(), which calls Sbot::init(KEYSTORE, IP_PORT, NET_ID) inside of it

Perfect, thanks for the tip. I'll do that.

I'll get some other PR's ready before merging this.

Haha yeah the turbofish is definitely one of the weirder pieces of Rust syntax. > the pattern I would probably use in the application code is to define a separate utility function with no arguments init_sbot(), which calls Sbot::init(KEYSTORE, IP_PORT, NET_ID) inside of it Perfect, thanks for the tip. I'll do that. I'll get some other PR's ready before merging this.

This pull request can be merged automatically.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

You can also view command line instructions.