Skip to content

Config#8

Closed
mkl- wants to merge 5 commits into
lightningnetwork:masterfrom
mkl-:config
Closed

Config#8
mkl- wants to merge 5 commits into
lightningnetwork:masterfrom
mkl-:config

Conversation

@mkl-
Copy link
Copy Markdown
Contributor

@mkl- mkl- commented Feb 23, 2016

Allow to config lnd using command line options as well as using configuration file.
Approach is very similar to approach used in btcd.
It seems that lnd still uses btcd in some form so I left configuration options for it.

Comment thread config.go
)

const (
defaultConfigFilename = "lnwallet.conf"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The name of the config file should instead be: "lnd.conf". lnwallet has it's own config struct, be we'll only require users to fill out a single concrete configuration file.

@Roasbeef
Copy link
Copy Markdown
Member

Hey @mkl-, thanks for working with me in order to get morph your previous PR into something merge-able! This looks pretty good to me, but I have a few comments I'd like addressed before I merge this in.

I've completed a preliminary review, but I have yet to test this branch against the current state of master yet.

Comment thread config.go
defaultBTCDHost = "localhost:18334"
defaultBTCDUser = "user"
defaultBTCDPass = "passwd"
defaultBTCDCACertPath = ""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be safely set with something along the lines of:

btcdHomeDir = btcutil.AppDir("btcd", false)
defaultBTCDCACertPath = filepath.Join(btcdHomeDir, "rpc.cert")

@Roasbeef
Copy link
Copy Markdown
Member

Hey @mkl-, thanks for starting the work to add a config to lnd! I've squashed+merged your commits within this PR in e07086a, which is now checked into master.

Stepping off your shoulders, I've also finalized the logging, configuration, and signal handling functionality within lnd.

Closing this now, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants