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

Update the zap logger dependency #9055

Merged
merged 1 commit into from
Nov 11, 2017
Merged

Update the zap logger dependency #9055

merged 1 commit into from
Nov 11, 2017

Conversation

jsternberg
Copy link
Contributor

@jsternberg jsternberg commented Nov 3, 2017

The previous sha was taken from a revision on a devel branch that I
thought would continue staying in the tree after it was merged. That
revision was rebased away and the API was changed for the logger.

This updates the usage of the logger and adds a simple package for
constructing the base logger.

  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

@ghost ghost assigned jsternberg Nov 3, 2017
@ghost ghost added the review label Nov 3, 2017
@jsternberg jsternberg force-pushed the js-update-zap-logger branch 3 times, most recently from 30c4605 to 43a2622 Compare November 4, 2017 03:26
@jsternberg jsternberg force-pushed the js-update-zap-logger branch 2 times, most recently from b7dc1b6 to 3eabc6e Compare November 10, 2017 22:02
Copy link
Contributor

@mark-rushakoff mark-rushakoff left a comment

Choose a reason for hiding this comment

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

One small change requested, then should be good to merge.

zap.NewTextEncoder(),
zap.Output(os.Stderr),
)
cmd.Logger = logger.New(os.Stderr)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be cmd.Stderr.

The previous sha was taken from a revision on a devel branch that I
thought would continue staying in the tree after it was merged. That
revision was rebased away and the API was changed for the logger.

This updates the usage of the logger and adds a simple package for
constructing the base logger.

The 1.0 version of zap changed the format of the default console logger
so this change moves over to this new logger instead of attempting to
retain backwards compatibility with the old format.
@jsternberg
Copy link
Contributor Author

Race tests are the ones that failed again. I'm going to merge this anyway.

@jsternberg jsternberg merged commit 7cae889 into master Nov 11, 2017
@ghost ghost removed the review label Nov 11, 2017
@jsternberg jsternberg deleted the js-update-zap-logger branch November 11, 2017 04:01
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.

2 participants