Skip to content
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

New be within matcher and RSpec.deprecate fix #32

Closed
wants to merge 0 commits into from

Conversation

myronmarston
Copy link
Member

This matcher is much nicer than be_close, in my opinion. It should probably only be included in the future 2.1.0 release since it's a new API.

The first commit should probably also be included in the next 2.0.x release, since RSpec.deprecate is already being called here, even though it's defined in rspec-core and we want rspec-expectations to be usable without core.

@dchelimsky
Copy link
Contributor

I merged the deprecate port. Thinking about depending on the deprecated gem in the future, though. WDYT?

@myronmarston
Copy link
Member Author

Add be_within(delta).of(expected) matcher

  • Delegate be_close(expected, delta) to be_within
  • Deprecate be_close(expected, delta)
  • Reads much better.
  • The argument ordering is clearer.
  • Closed by 7852dbd.

@myronmarston
Copy link
Member Author

Thinking about depending on the deprecated gem in the future, though. WDYT?

I've never used the deprecated gem, but it looks like it does provide nice, flexible deprecation functionality. That said, my feeling is that the current deprecation behavior is pretty sufficient, and I don't anticipate RSpec.deprecate needing much maintenance in the future. It does the simple thing it's intended to do well.

If/when you find that you need more deprecation flexibility than the current RSpec.deprecate, I think it would make sense to move to the deprecated gem rather than make the same updates to the method in core and expectations...but for now I don't think it's worth adding the extra dependency.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants