Improve configgen command line handling a bit #440
Conversation
|
Feedback (especially w.r.t naming) much appreciated. |
Codecov Report
@@ Coverage Diff @@
## master #440 +/- ##
==========================================
- Coverage 74.54% 74.46% -0.09%
==========================================
Files 83 83
Lines 10550 10556 +6
==========================================
- Hits 7865 7861 -4
- Misses 2685 2695 +10
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
dpc
force-pushed
the
configgen
branch
2 times, most recently
from
83f8a68
to
c5b457e
Compare
|
Concept ACK. Much better! Consuming this script right now as well. It makes sense for these values to have sane defaults as well. Perhaps the ones in build.sh make sense. I'm not sure what "hbbft" and "api" mean, but I'm just getting up to speed so maybe those names are good. |
|
Also, what unit are the denominations? msat? That should be labeled too |
|
@DanGould Just seen some confusion on #general w.r.t units somewhere. 👍 on including units here and everywhere. |
|
Ports and denominations should have a default. |
|
fyi esplora uses default ports 5000 & 5001. Best to avoid those. Esplora pairs nicely with fedimint. |
|
There's 63k ports to choose from and it always have to be 8000, 8080, 3000 or some other x000. 🤷 |
fedimint/src/bin/configgen.rs
Outdated
| amount_tiers: Vec<Amount>, | ||
|
|
||
| /// Available denominations of notes issues by the federation (comma separated) | ||
| #[clap(long = "denominations", value_delimiter = ',')] |
There was a problem hiding this comment.
Can we enforce that there has to be at least one denomination? Forgetting to specify them bit me a few times …
I would have assumed that this is the default behavior, but apparently it isn't (I just tested it again) and there has been a lengthy discussion about it (didn't read it all) clap-rs/clap#1772
dpc
force-pushed
the
configgen
branch
2 times, most recently
from
1865e72
to
73093ca
Compare
Positional arguments are not descriptive and harder to use.
|
|
| num_nodes: u16, | ||
|
|
||
| /// Base hbbft port | ||
| #[clap(long = "hbbft-base-port", default_value = "17240")] |
There was a problem hiding this comment.
I checked that the port is out of the range for ephemeral ports on most systems (32768–60999 for Linux apparently).
Positional arguments are not descriptive and harder to use.