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

Logging methods (debug, info, warn, error, fatal) all return nil #27

Merged

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Nov 21, 2017

This PR makes logger.debug "Hello" return nil.

Why would I need this?

Your existing code rescued and logged an exception using warn. The return value of that invocation was nil.

Someone comes along to upgrade that warn to use a configurable mixlib-log logger. The return value of that logged exception is now non-nil.

@olleolleolle olleolleolle requested a review from a team November 21, 2017 15:41
@olleolleolle olleolleolle changed the title Logging method return nil Logging methods (debug, info, warn, error, fatal) all return nil Nov 21, 2017
@olleolleolle olleolleolle force-pushed the feature/logger-methods-to-return-nil branch 2 times, most recently from 36b4707 to a339b37 Compare November 21, 2017 15:48
  - Fixes chef#21

Signed-off-by: Olle Jonsson <[email protected]>
@thommay thommay force-pushed the feature/logger-methods-to-return-nil branch from a339b37 to 7be3c2e Compare February 28, 2018 14:37
@thommay thommay merged commit 38b2ec3 into chef:master Feb 28, 2018
@thommay
Copy link
Contributor

thommay commented Feb 28, 2018

@olleolleolle thanks so much for contributing this, and sorry it took so long to get merged. It'll ship with Chef Client 14 next month 🎉

@olleolleolle olleolleolle deleted the feature/logger-methods-to-return-nil branch February 28, 2018 14:40
@olleolleolle
Copy link
Contributor Author

\o/ Just sitting down reading the Chef versioning RFC to figure out how to make my puny V0 client stay on that.

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