-
Notifications
You must be signed in to change notification settings - Fork 68
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
airbrake-ruby: fix add_filter block API #10
Conversation
5696a0c
to
79dd070
Compare
Fixes #8 (add_filter throwing exception)
79dd070
to
d5f0939
Compare
@@ -6,7 +6,7 @@ | |||
end | |||
|
|||
before do | |||
described_class.configure do |c| | |||
@notifier = described_class.configure do |c| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be more consistent as a let
or subject
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let
or subject
cache their values, don't they? Here we need to reset the notifier to make sure that we always use a fresh one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cached only within that test https://www.relishapp.com/rspec/rspec-core/v/2-11/docs/helper-methods/let-and-let
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks. Replaced with let!
. PTAL
PTAL |
@@ -5,12 +5,14 @@ | |||
'https://airbrake.io/api/v3/projects/113743/notices?key=fd04e13d806a90f96614ad8e529b2822' | |||
end | |||
|
|||
before do | |||
let!(:notifier) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need the !
, here, unless I'm missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise some tests start failing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see it now :-)
LGTM |
airbrake-ruby: fix add_filter block API
Fixes #8 (add_filter throwing exception)