Provide upgrade warning for pending behaviour change. #1268
Conversation
There was a problem hiding this comment.
I've never really liked using :: for message sends (or in doc strings to indicate a method). I've never asked David about it but I think he liked it quite a bit since there are a fair number of places in rspec that use it. I always prefer a . for a message send though; to me, :: is a constant scoping operator and I don't see a benefit for using it for something else even though ruby allows it.
Are there situations where you prefer it?
(Don't bother changing it in this PR -- since it's in 2.99 and not master it's not sticking around and I don't really care. I'm just bringing this up because I'm curious how others on the team feel about it).
There was a problem hiding this comment.
I prefer ., I used :: here to be consistent with the other specs around it.
|
This LGTM, but the build is broken for some reason. Regardless, we should hold off merging until the 3.0 feature PR is ready to merge. Thanks for taking the initiative to have this ready, though! |
|
oh yes, build failure is unrelated to this PR, I see it on |
|
Created a separate issue for the build failures: #1269 |
|
Green now. |
|
Since |
|
We'll have to add a deprecation for |
|
Both cases now warn and have skip available. Short of issues in the new code, this should be ready to merge. |
lib/rspec/core/example_group.rb
Outdated
There was a problem hiding this comment.
Not a merge blocker, but should this be "Backported from RSpec 3 to aid migration"? "Backported from 3" is a little odd on its own.
|
Addressed both issues. |
|
This LGTM. Merge whenever you like. |
|
Holding off merge until #1267 is pulled in. |
|
HTML fixtures need re-gen ... am hoping to be able to rebase this on top of #1306 to solve that problem. |
Includes a backport of `skip` to assist with upgrading.
|
Build failure is rbx. |
lib/rspec/core/configuration.rb
Outdated
There was a problem hiding this comment.
Seems like you should remove this call to add_setting since you are defining the methods below...
There was a problem hiding this comment.
add_setting does some other stuff too.
|
Pushed up response to comment, I'm happy for this to merged on green (if others are too). |
|
LGTM |
|
Failure is due to rubinius/rubinius#2934 so I'm merging. |
Provide upgrade warning for pending behaviour change.
…-deprecation Provide upgrade warning for pending behaviour change. --- This commit was imported from rspec/rspec-core@733b423.
2 participants
Warning for #1267.