-
-
Notifications
You must be signed in to change notification settings - Fork 10k
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
rubocop: features/step_definitions.rb #4956
Conversation
@@ -14,24 +14,24 @@ | |||
|
|||
# | |||
|
|||
Given %r{^I have a blank site in "(.*)"$} do |path| | |||
if !File.exist?(path) | |||
Given(/^I have a blank site in "(.*)"$/) do |path| |
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.
This should still fail, we use %r!!
over //
or %r{}
.
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.
Hm, actually it failed with %r!!, I guess because we have following in .rubocop.yml
Style/RegexpLiteral:
EnforcedStyle: slashes
Should I change it to percent_r
?
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'll digress to @parkr but we've been moving to %r!!
all over the place.
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.
Only when there is a literal /
in the regex
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.
Which is to say, /test/
is fine, but /test\//
is ugly so in that case we use %r!test/!
instead.
Thanks! Just a few comments. |
LGTM |
Thank you so much! @jekyllbot: merge +dev |
Fixes in features/step_definitions.rb
#4885