Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: double format issue #63

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: double format issue #63

wants to merge 2 commits into from

Conversation

ghostoy
Copy link

@ghostoy ghostoy commented Jul 18, 2022

splat of winston takes care of message formatting. No need to format
the message on @midwayjs/logger side.

Fixed #62

@czy88840616
Copy link
Member

我先把测试处理下。

@czy88840616
Copy link
Member

@ghostoy 可以先 rebase 一下 master

`splat` of `winston` takes care of message formatting. No need to format
the message on `@midwayjs/logger` side.

Fixed midwayjs#62
@ghostoy
Copy link
Author

ghostoy commented Jul 28, 2022

@czy88840616 rebase过了

@czy88840616
Copy link
Member

@ghostoy 貌似ci挂了 。

@ghostoy
Copy link
Author

ghostoy commented Jul 31, 2022

@czy88840616 应该好了,再跑下ci试试

@czy88840616
Copy link
Member

应该是有影响原来的 case 了

@ghostoy
Copy link
Author

ghostoy commented Aug 2, 2022

现在test里期望logger.info('first', 'second')输出first second,而logger.info('%s', 'second')输出second。这个和上游使用的winston的log接口行为不一致。

还有下面这些输出在原有实现中是没有定义和测试的:

  • logger.info('%s')会输出{ [Symbol(origin_args)]: [Array] }
  • logger.info('%%')会输出%%,而logger.info('%%', 1)会输出% 1
  • logger.info('%%d', 999)会输出NaN 999

这个PR我暂时不打算继续改了,对于%的输出实在不好兼容现有的测试用例。

@czy88840616
Copy link
Member

logger.info('first', 'second') 是兼容原有 egg-logger 的需求,和 winston 不一定很兼容(后续大概率会抛弃 winston,自己来实现)。

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

Successfully merging this pull request may close these issues.

日志输出被格式化两遍
2 participants