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

Better logging with winston #1234

Merged
merged 2 commits into from
Mar 29, 2016
Merged

Better logging with winston #1234

merged 2 commits into from
Mar 29, 2016

Conversation

flovilmart
Copy link
Contributor

Centralizes all logs (including verbose) through winston.

Makes all logs available through the API

@codecov-io
Copy link

Current coverage is 92.99%

Merging #1234 into master will decrease coverage by -0.01% as of 90554d5

@@            master   #1234   diff @@
======================================
  Files           83      84     +1
  Stmts         5287    5268    -19
  Branches       967     958     -9
  Methods          0       0       
======================================
- Hit           4917    4899    -18
- Partial         10      11     +1
+ Missed         360     358     -2

Review entire Coverage Diff as of 90554d5

Powered by Codecov. Updated on successful CI builds.

@@ -254,7 +263,7 @@ class ParseServer {
if (!process.env.TESTING) {
process.on('uncaughtException', (err) => {
if ( err.code === "EADDRINUSE" ) { // user-friendly message for this common error
console.log(`Unable to listen on port ${err.port}. The port is already in use.`);
log.error(`Unable to listen on port ${err.port}. The port is already in use.`);
Copy link
Contributor

Choose a reason for hiding this comment

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

This one I think actually should be console.log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright!

@drew-gross
Copy link
Contributor

Looks good to me aside from the one thing. @peterdotjs you wrote most of this code, want to take a look?

@peterdotjs
Copy link
Contributor

LGTM.

@flovilmart
Copy link
Contributor Author

Merge when you like :)

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

Successfully merging this pull request may close these issues.

5 participants