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

Loggers should accept both numbers and symbols #6967

Merged
merged 3 commits into from
May 1, 2018
Merged

Conversation

pathawks
Copy link
Member

From #6305:

Jekyll's logger should start trying to conform to standard logging practices in Ruby. That said, the logger level should accept both a :symbol (if you really want,) and a number, this way users aren't taken aback when they know what log level they want. This commit makes it so that the logger level can be set normally and within Jekyll's own constraints.

Jekyll.logger.log_level = 0
Jekyll.logger.log_level = Logger::DEBUG
Jekyll.logger.log_level = :debug

Jekyll's logger should start trying to conform to standard logging practices in Ruby.  That said, the logger level should accept both a `:symbol` (if you really want,) and a number, this way users aren't taken aback when they know what log level they want.  This commit makes it so that the logger level can be set normally and within Jekyll's own constraints.

```ruby
Jekyll.logger.log_level = 0
Jekyll.logger.log_level = Logger::DEBUG
Jekyll.logger.log_level = :debug
```
@pathawks pathawks requested review from a team April 30, 2018 16:03
@pathawks pathawks added this to the 4.0 milestone Apr 30, 2018
@ashmaroli
Copy link
Member

@pathawks How does this benefit our regular users or plugin developers..?

@pathawks
Copy link
Member Author

@ashmaroli I think the benefit here is that it makes our logger behave more like standard Ruby loggers.

@ashmaroli
Copy link
Member

okay.. will review after the tests pass..

@pathawks
Copy link
Member Author

@ashmaroli Travis is now ✅

@ashmaroli ashmaroli requested a review from parkr May 1, 2018 03:58
@ashmaroli
Copy link
Member

(explicitly requesting a review from @parkr (instead of a team) because something about this doesn't feel right to me..)

@DirtyF
Copy link
Member

DirtyF commented May 1, 2018

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 9461c90 into master May 1, 2018
@jekyllbot jekyllbot deleted the envygeeks-patch-1 branch May 1, 2018 12:37
jekyllbot added a commit that referenced this pull request May 1, 2018
@jekyll jekyll locked and limited conversation to collaborators Jul 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants