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! 😱
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! 😱_Originally posted by @miparnisari in https://github.com/mailgun/gubernator/pull/214#discussion_r1461316114_