Added listen port option to the install command. #369
Conversation
There was a problem hiding this comment.
Hi @Hwatwasthat thanks for your first contribution to innernet and sorry for taking such a long time to respond.
This looks great I only have two little suggestions. Also have you tested this actually works? .)
Fixing the failed CI should be just a matter of rebasing on latest main.
| innernet_shared::ensure_dirs_exist(&[&opts.config_dir])?; | ||
| let config = InterfaceConfig::from_file(invite)?; | ||
| let config = InterfaceConfig::from_file(invite).map(|mut config| { | ||
| config.interface.listen_port = listen_port; |
There was a problem hiding this comment.
Invite files currently don't specify listen ports. We don't expect them to in the future but out of caution I'd feel safer if this overrode the config only when the listen_port argument is Some.
There was a problem hiding this comment.
I'll give it an update shortly so it only puts it in if there is actually a value.
Remove the short option, only allow using --listen-port. Co-authored-by: Jen <jentak@hey.com>
|
I'll rebase in a bit on main to make it happy. I've tested it but obviously after the little changes and rebase I'll need to give it a go again! No worries about the delay, I was a bit surprised when I saw the email because I might have forgotten sending in the PR to be honest! |
2 participants
Resolves #346. This follows the argument layout and text for set-interface. I think this is the minimum change needed to add this, but this is my first contribution here so please set me straight if I'm getting things wrong!