Restructure project #10
Conversation
log/log.go
Outdated
| } | ||
|
|
||
| func newLogrusLogger(cfg config.Provider) *logrus.Logger { | ||
|
|
log/log.go
Outdated
| } | ||
|
|
||
| func NewLogger(cfg config.Provider) *Logger { | ||
| return newLogrusLogger(cfg) |
There was a problem hiding this comment.
Anything calls newLogrusLogger? Make it simpler just inline the method
There was a problem hiding this comment.
Line 18. This function is created to generate a new logger.
Developer can simply import github.com/hyperpilotio/ingestor/log and use those function ( ex: log.Info ) directly because the init function has been executed. If they want to have a new logger for some purposes, they can call log.NewLogger to get a new logger.
https://github.com/Hyperpilotio/ingestor/pull/10/files#diff-fbf850c1cb9fc6234051c4e9517a5274R18
version.go
Outdated
| var GitCommit string | ||
|
|
||
| // The main version number that is being run at the moment. | ||
| const Version = "0.1.0" |
There was a problem hiding this comment.
Makefile
VERSION := $(shell grep "const Version " version.go | sed -E 's/.*"(.+)"$$/\1/')
docker build --build-arg VERSION=${VERSION} --build-arg GIT_COMMIT=$(GIT_COMMIT) -t $(IMAGE_NAME):local .
...
There was a problem hiding this comment.
hmm, seems quite hacky for me to grep a file for a version.
I rather make it simple for now and just remove this so we don't end up having to maintain this when we never needed it. Git commit makes a lot more sense for me.
|
After these fixed we can merge |
swhsiang
force-pushed
the
restructure
branch
from
fc2b677 to
0c19b2f
Compare
WHY / WHAT
Instead of passing configuration object into each function. Initializing the configuration object in config/config.go. Those function which requires configuration can import
github.com/hyperpilotio/ingestor/configand the function would be able to use the configuration object.