keon94 commented on code in PR #2656:
URL: https://github.com/apache/incubator-devlake/pull/2656#discussion_r941943051


##########
logger/logger.go:
##########
@@ -75,32 +95,84 @@ func (l *DefaultLogger) Error(format string, a 
...interface{}) {
        l.Log(core.LOG_ERROR, format, a...)
 }
 
-// bind two writer to logger
-func (l *DefaultLogger) Nested(name string) core.Logger {
-       writerStd := os.Stdout
-       fileName := ""
-       loggerPrefixRegex := regexp.MustCompile(`(\[task #\d+]\s\[\w+])`)
-       groups := loggerPrefixRegex.FindStringSubmatch(fmt.Sprintf("%s [%s]", 
l.prefix, name))
-       if len(groups) > 1 {
-               fileName = groups[1]
+func (l *DefaultLogger) Nested(newPrefix string, config ...*core.LoggerConfig) 
core.Logger {

Review Comment:
   If you want Nested() to modify the existing logger rather than returning a 
new one, then we're irreversibly changing state on the logger. And as we pass 
this logger to other functions its state keeps getting changed and the behavior 
gets out of control. I think Nested() should return a new logger instance that 
inherits the properties of the original (like a fork()), but is detached from 
it. The change that I agree with you here is a SetStream API, I think that's a 
good idea.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to