feat: SetupDaemonConfig no longer needs a file #214
Conversation
| func SetupDaemonConfig(logger *logrus.Logger, configFile string) (DaemonConfig, error) { | ||
| // SetupDaemonConfig returns a DaemonConfig object that is the result of merging the lines | ||
| // in the provided configFile and the environment variables. See `example.conf` for all available config options and their descriptions. | ||
| func SetupDaemonConfig(logger *logrus.Logger, configFile io.Reader) (DaemonConfig, error) { |
There was a problem hiding this comment.
This function really really shouldn't accept a concrete instance of a logrus.Logger. This should be an interface and, if consumers want, they can pass their own implementation (or a NoOpLogger)
Something else I noticed: it is really really easy for consumers of this project to simply do gubernator.SpawnDaemon(ctx, DaemonConfig{GRPCListenAddress: "localhost:8111"}) - this will set all other configs to their zero values! 😱
There was a problem hiding this comment.
That sounds like a great idea! Thank you!
There was a problem hiding this comment.
Agreed on the Logrus concrete type thing. Would be a nice addition to Gubernator v3 to ditch Logrus once and for all.
miparnisari
force-pushed
the
SetupDaemonConfig
branch
from
37cdd00 to
bdbb347
Compare
| func SetupDaemonConfig(logger *logrus.Logger, configFile string) (DaemonConfig, error) { | ||
| // SetupDaemonConfig returns a DaemonConfig object that is the result of merging the lines | ||
| // in the provided configFile and the environment variables. See `example.conf` for all available config options and their descriptions. | ||
| func SetupDaemonConfig(logger *logrus.Logger, configFile io.Reader) (DaemonConfig, error) { |
There was a problem hiding this comment.
That sounds like a great idea! Thank you!
Currently,
SetupDaemonConfigneeds a concrete instance oflogrusand a file. This PR removes the need of a file. It's bad practice to accept a filename as parameter: https://100go.co/?h=file#using-a-filename-as-a-function-input-46. If consumers want, they can pass a string.