Remove Set #1870
Remove Set #1870
Conversation
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
This seems like an odd place for it given it's not really "core" to RSpec. It's very much a supporting class and not something that should be front-and-center in the main required file of RSpec. Can you move this into another file?
|
Looking good. It concerns me a little that |
|
Yeah this was a proof of concept really, I think it needs it's own file and specs. |
|
I'll try to do that sometime tomorrow |
JonRowe
force-pushed
the
remove_set
branch
3 times, most recently
from
564180c to
854f5a3
Compare
|
Think this is ready for a review... |
|
|
||
| def <<(key) | ||
| @values[key] = true | ||
| end |
There was a problem hiding this comment.
With a normal set (and every other built-in that defines a << method that I can think of), this method returns the set itself:
irb(main):002:0> s = Set.new
=> #<Set: {}>
irb(main):003:0> s << 1
=> #<Set: {1}>
Yours returns true, I believe. I think you should add self on the next line to mirror the normal behavior.
|
can you remove this as well? rspec-core/spec/rspec/core_spec.rb Line 7 in 850b904 |
JonRowe
force-pushed
the
remove_set
branch
2 times, most recently
from
d32bb27 to
0cbb17f
Compare
|
Removed and renamed... |
| @@ -2,6 +2,8 @@ | |||
| $_rspec_core_load_started_at = Time.now | |||
| # rubocop:enable Style/GlobalVars | |||
|
|
|||
| require 'rbconfig' | |||
There was a problem hiding this comment.
Why did you add back this require?
There was a problem hiding this comment.
I didn't intentionally... rebase...
…upset which uses a hash
…than `#each_key`
|
LGTM, merge when green. |
|
Merging despite Appveyor failure because it's a gem fetch error and the other appveyor job passed |
Remove Set
[skip ci]
In general we want to require as little of the std lib as possible to prevent false positives with peoples code due to us using part of the std lib and then developers not requiring those parts, this removes our existing usage of set which is normally done for uniqueness / constant time lookup reasons and replaces it with a class that uses a hash internally.