search

0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup
MIT License
27 stars 23 forks source link

feat: add flags to init command to enable non-interactive initilization #299

Closed mFragaBA closed 2 months ago

mFragaBA commented 2 months ago

closes #288

bobbinth commented 2 months ago

Did we decide not to do something like

miden-clinet init --rpc devnet.miden.io
miden-clinet init --rpc testnet.miden.io
mFragaBA commented 2 months ago

Did we decide not to do something like

miden-clinet init --rpc devnet.miden.io
miden-clinet init --rpc testnet.miden.io

Oh, do you mean we should either add the --deafult/testnet flags or --rpc but not both to the config?

bobbinth commented 2 months ago

yes, that's what i was thinking - if we have --rpc we don't really need the default/testnet flags (we can also have a default for --rpc flag).

mFragaBA commented 2 months ago

yes, that's what i was thinking - if we have --rpc we don't really need the default/testnet flags (we can also have a default for --rpc flag).

should we also consider the protocol (http/https which I think it's actually gRPC/gRPC + tls) in this flag? Or is it safe to default to http?

bobbinth commented 2 months ago

should we also consider the protocol (http/https which I think it's actually gRPC/gRPC + tls) in this flag? Or is it safe to default to http?

Unless specified, I would probably default to https.

mFragaBA commented 2 months ago

should we also consider the protocol (http/https which I think it's actually gRPC/gRPC + tls) in this flag? Or is it safe to default to http?

Unless specified, I would probably default to https.

Ok! I added the possibility to specify the protocol but if not provided it defaults to https. Thanks for the input!

bobbinth commented 2 months ago

I think the overall changes look good to me. The only thing that I would suggest is that I think I'd prefer it if we would not default to making the command interactive in some cases. For example, if we do miden-client init --rpc testnet.miden.io I think it would be nice if, for other settings (ie the store) we would set the defaults instead of prompting the user. Or maybe we could provide a --default flag that sets defaults for anything that was not specified?

I think we could have defaults for all settings (--rpc included, which could default to testnet.miden.io).

mFragaBA commented 2 months ago

I think the overall changes look good to me. The only thing that I would suggest is that I think I'd prefer it if we would not default to making the command interactive in some cases. For example, if we do miden-client init --rpc testnet.miden.io I think it would be nice if, for other settings (ie the store) we would set the defaults instead of prompting the user. Or maybe we could provide a --default flag that sets defaults for anything that was not specified?

I think we could have defaults for all settings (--rpc included, which could default to testnet.miden.io).

in that case we wouldn't have an interactive init, right?

bobbinth commented 2 months ago

in that case we wouldn't have an interactive init, right?

Correct!

mFragaBA commented 2 months ago

in that case we wouldn't have an interactive init, right?

Correct!

Done! Will use default values unless specified (also I'd rather use localhost as the default instead of testnet.miden.io)

bobbinth commented 2 months ago

also I'd rather use localhost as the default instead of testnet.miden.io

I think that's fine.