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

Add the ability to yield a message. #6306

Closed
wants to merge 1 commit into from
Closed

Add the ability to yield a message. #6306

wants to merge 1 commit into from

Conversation

envygeeks
Copy link
Contributor

Standard loggers in Ruby mostly yield their messages (this is the default behavior of Ruby's own logger.) Or if there is a block then the message is the topic, a colon is automatically added, and the message is shipped with the topic. Jekyll should conform to this behavior so that it's logger can be passed to libraries that expect this standard behavior.

[1] pry(main)> Jekyll.logger.info("Just so you know") # => Just so you know 
[2] pry(main)> Jekyll.logger.info("Just so you know", "this is a message") # => Just so you know this is a message
[3] pry(main)> Jekyll.logger.info("Just so you know") { "this is a message" }
  # => Just so you know: this is a message

Standard loggers in Ruby mostly `yield` their messages (this is the default behavior of Ruby's own logger.) Or if there is a block then the message is the topic, a colon is automatically added, and the message is shipped with the topic.  Jekyll should conform to this behavior so that it's logger can be passed to libraries that expect this standard behavior.

```ruby
[1] pry(main)> Jekyll.logger.info("Just so you know") # => Just so you know 
[2] pry(main)> Jekyll.logger.info("Just so you know", "this is a message") # => Just so you know this is a message
[3] pry(main)> Jekyll.logger.info("Just so you know") { "this is a message" }
  # => Just so you know: this is a message
```
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

This is great! Just 2 small changes that should fix the CI errors.

@@ -79,7 +79,7 @@ def warn(topic, message = nil)
#
# Returns nothing
def error(topic, message = nil)
writer.error(message(topic, message))
writer.error(message(topic, message, &block))
Copy link
Member

Choose a reason for hiding this comment

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

Need to add this to the method signature on line 81

@@ -69,7 +69,7 @@ def info(topic, message = nil)
#
# Returns nothing
def warn(topic, message = nil)
writer.warn(message(topic, message))
writer.warn(message(topic, message, &block))
Copy link
Member

Choose a reason for hiding this comment

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

Need to add this to the method signature on line 71 (&block)

@parkr
Copy link
Member

parkr commented Aug 17, 2017

The original repository is gone so no edits can be made. Rolled this change into #6315.

@parkr parkr closed this Aug 17, 2017
@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.

None yet

3 participants