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

Configurable Logger #2444

Merged
merged 3 commits into from May 30, 2014

Conversation

Projects
None yet
3 participants
@createdbypete
Contributor

createdbypete commented May 24, 2014

Allows Jekyll's logger to be set to any Logger compatible instance.

  • Stevenson now inherits from Logger and extends some methods to restore passing tests.
  • New LogWriter class provides an interface between Jekyll and Logger to maintain Jekyll's logging API.

When using Jekyll as a library this allows you to change the logger, for example to output to a file or whatever you want to do so long as it's compatible with Loggers API.

createdbypete added some commits May 24, 2014

Allow Jekyll's logger to be set to any Logger compatible instance
* Stevenson now inherits from Logger and extends some methods to use $stderr
for log messages greater than info level.
* LogWriter provides an interface between Jekyll and Logger to maintain API.
Renaming LogWriter to LogAdapter and adding tests for class
String coloring moved to Stevenson as not responsibility of LogAdapter
@parkr

View changes

Show outdated Hide outdated lib/jekyll/log_adapter.rb
@createdbypete

This comment has been minimized.

Show comment
Hide comment
@createdbypete

createdbypete May 25, 2014

Contributor

@parkr thanks for taking the time to provide feedback. Fixed up the styling and constants as mentioned.

Contributor

createdbypete commented May 25, 2014

@parkr thanks for taking the time to provide feedback. Fixed up the styling and constants as mentioned.

@logdev.puts(
format_message(format_severity(severity), Time.now, progname, message))
true
end

This comment has been minimized.

@parkr

parkr May 26, 2014

Member

What is the reason for this method? Don't we get this for free from ::Logger?

@parkr

parkr May 26, 2014

Member

What is the reason for this method? Don't we get this for free from ::Logger?

This comment has been minimized.

@createdbypete

createdbypete May 26, 2014

Contributor

Almost, I added this line (15) to switch to $stderr based on severity (see #set_logdevice). Then on line 29 I used #puts instead of #write to keep passing tests.

My intention was to avoid changing any tests so that everything continued to work as before, Stevenson still matches the Logger API but a few bits about how Logger would do things were not compatible with what the Jekyll tests expected.

This could be cleaned up if I updated the tests to stub on $stdout #write method instead of #puts then I could call super. We could clean up further if the output was to only one stream (such as $stdout) regardless of severity, that way I'd only need to set the formatter in #initialize.

@createdbypete

createdbypete May 26, 2014

Contributor

Almost, I added this line (15) to switch to $stderr based on severity (see #set_logdevice). Then on line 29 I used #puts instead of #write to keep passing tests.

My intention was to avoid changing any tests so that everything continued to work as before, Stevenson still matches the Logger API but a few bits about how Logger would do things were not compatible with what the Jekyll tests expected.

This could be cleaned up if I updated the tests to stub on $stdout #write method instead of #puts then I could call super. We could clean up further if the output was to only one stream (such as $stdout) regardless of severity, that way I'd only need to set the formatter in #initialize.

$stderr
else
$stdout
end

This comment has been minimized.

@parkr

parkr May 26, 2014

Member

Should this set a variable? It seems only to fetch the log device.

@parkr

parkr May 26, 2014

Member

Should this set a variable? It seems only to fetch the log device.

@parkr parkr referenced this pull request May 30, 2014

Closed

Release Jekyll 2.1.0 #2465

5 of 7 tasks complete

@parkr parkr merged commit 83fb1fd into jekyll:master May 30, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

parkr added a commit that referenced this pull request May 30, 2014

@createdbypete createdbypete deleted the createdbypete:configurable-logger branch Jun 9, 2014

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.