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

feat: add config.logger.disableConsoleAfterReady #1001

Merged
merged 6 commits into from
Jun 12, 2017

Conversation

fengmk2
Copy link
Member

@fengmk2 fengmk2 commented Jun 5, 2017

let app can change the logger config

Checklist
  • npm test passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change

let app can change the logger config
@codecov
Copy link

codecov bot commented Jun 5, 2017

Codecov Report

Merging #1001 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1001   +/-   ##
=======================================
  Coverage   98.96%   98.96%           
=======================================
  Files          28       28           
  Lines         675      675           
=======================================
  Hits          668      668           
  Misses          7        7
Impacted Files Coverage Δ
config/config.default.js 100% <ø> (ø) ⬆️
lib/core/logger.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d705e4...bae18b6. Read the comment docs.

@@ -213,6 +214,7 @@ module.exports = appInfo => {
env: appInfo.env,
level: 'INFO',
consoleLevel: 'INFO',
disableConsoleAfterReady: appInfo.env !== 'local' && appInfo.env !== 'unittest',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

文档中没写有 env,是文档少了?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

恩,就在 env 章节写从 app.config.env 获取好了。

@popomore
Copy link
Member

popomore commented Jun 6, 2017

logger 真是个复杂的东西。

@fengmk2
Copy link
Member Author

fengmk2 commented Jun 8, 2017

@popomore @dead-horse 可以合并了?

@popomore
Copy link
Member

popomore commented Jun 9, 2017

再重跑下 ci

@atian25
Copy link
Member

atian25 commented Jun 9, 2017

  1) test/lib/plugins/development.test.js reload workers should reload when file changed:
      AssertionError: should match stdout expected `/app_worker#2:\d+ started/(RegExp)` but actual `2017-06-09 05:21:20,641 INFO 3946 [master] egg version 1.4.0\n2017-06-09 05:21:21,139 INFO 3946 [master] agent_worker#1:3952 started (489ms)\n2017-06-09 05:21:21,870 INFO 3946 [master] egg started on http://127.0.0.1:17024 (1227ms)\n2017-06-09 05:21:22,123 WARN 3952 [agent:development] reload worker because /home/travis/build/eggjs/egg/test/fixtures/apps/reload-worker/app/controller/home.js change\n2017-06-09 05:21:22,807 INFO 3946 [master] app_worker#1:3962 disconnect, suicide: true, state: disconnected, current workers: ["2"]\n(String)`
      + expected - actual
      -false
      +true

@popomore
Copy link
Member

popomore commented Jun 9, 2017

在这里解决 #1022 (comment)

@fengmk2
Copy link
Member Author

fengmk2 commented Jun 10, 2017

继续重跑

@popomore popomore merged commit f1b510c into master Jun 12, 2017
@popomore popomore deleted the disableConsoleAfterReady branch June 12, 2017 10:17
@popomore
Copy link
Member

要发版本么

@fengmk2
Copy link
Member Author

fengmk2 commented Jun 12, 2017

先不发,等需要发版本的时候带出去

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

Successfully merging this pull request may close these issues.

4 participants