-
Notifications
You must be signed in to change notification settings - Fork 247
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
Plumb Silencer.new and use it #542 spec #544
Plumb Silencer.new and use it #542 spec #544
Conversation
@ElMassimo Here's the follow-up PR from #542. Look good? |
ffc0608
to
2750b38
Compare
Excellent, thanks Colin! |
Merging this now since few tests are broken without it. |
BTW, I'm expecting to release this in v3.7.0 in the coming week. |
attr_accessor :only_patterns, :ignore_patterns | ||
|
||
def initialize | ||
configure({}) | ||
def initialize(options = {}) |
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'm not sure the backwards compatibility constraints but you should prefer **options
.
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.
All previous cases initialized this object with no arguments, so I don't think there's a concern there.
Can you help me understand what cases will work better with **options
vs. options = {}
?
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 know that **options
is effective at collecting the options hash as the last argument when there are many arguments. But with a single argument like this, I'm not seeing a difference between **options
and options = {}
?
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.
Maybe **options
is preferred because it always works regardless of other arguments before it?
I switched over to it here: 32bb3a7
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.
LGTM.
ignore!: [/\A\.ignored/]
silencer option inrecord_spec
so we're not depending onSilencer
's defaults.Silencer
to allow its config to be passed intoinitialize
rather than mutated byconfigure
. Update some comments there about deprecating mutation.