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 Logger notifier #77

Merged
merged 1 commit into from
Sep 11, 2015
Merged

Add Logger notifier #77

merged 1 commit into from
Sep 11, 2015

Conversation

bolshakov
Copy link
Owner

No description provided.

@tfausak tfausak self-assigned this Sep 10, 2015

describe '#logger' do
it 'reads the logger' do
logger = Logger.new(STDOUT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer testing with StringIO instead of standard out and error. You can see some example usage of StringIO in the IO notifier spec.

@tfausak
Copy link
Collaborator

tfausak commented Sep 10, 2015

Thanks for opening this pull request! This looks like a good addition to Stoplight. If we do merge this, we will also need to update the notifiers section of the readme to include this notifier.

@bolshakov
Copy link
Owner Author

Hi thanks for feedback! I've just updated REAMDE and specs.

@@ -321,7 +334,7 @@ Stoplight:
# config/initializers/stoplight.rb
require 'stoplight'
Stoplight::Light.default_data_store = Stoplight::DataStore::Redis.new(...)
Stoplight::Light.default_notifiers += [Stoplight::Notifier::HipChat.new(...)]
Stoplight::Light.default_notifiers += [Stoplight::Notifier::Logger.new(Rails.logger)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Good catch!

@tfausak
Copy link
Collaborator

tfausak commented Sep 10, 2015

Once you fix the STDOUT and implicit subject in the specs, this is good to merge. Thanks again!

@tfausak tfausak merged commit 098bf50 into bolshakov:master Sep 11, 2015
@tfausak
Copy link
Collaborator

tfausak commented Sep 11, 2015

Released in v1.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants