Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Deprecation formatter #915

Merged
merged 21 commits into from
May 23, 2013
Merged

Deprecation formatter #915

merged 21 commits into from
May 23, 2013

Conversation

dchelimsky
Copy link
Contributor

Per convo in #912, print deprecations through a new DeprecationFormatter, and change the deprecation API to:

RSpec.deprecate("thing to be deprecated", :replacement => "it's replacement")
RSpec.warn_deprecation("custom message that is arguably better than the message generated by RSpec.deprecate for a specific scenario")

@fables-tales
Copy link
Member

Nice job David! This looks great.

@dchelimsky
Copy link
Contributor Author

FYI - I added a last minute addition to change a hash key (internal). This is ready to go AFAIC.

Background:
Given a file named "lib/foo.rb" with:
"""ruby
class Foo; def bar; RSpec.deprecate "Foo#bar"; end; end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the noise of combining what is usually on multiple lines (via semicolons) inhibits clarity here. What do you think about using a more normal code layout like this?

class Foo
  def bar
    RSpec.deprecate "Foo#bar"
  end
end

Alternately, if you don't want all 5 lines, you could just define it as a top-level method directly on main, and then call it from the example below. That'd cut it down to 3 lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e8c1c59

@myronmarston
Copy link
Member

👍 Fantastic work, David. I left a few questions, but I'm completely happy with it as is. I'll hold off merging for now in case there's any followup from my questions.

dchelimsky added a commit that referenced this pull request May 23, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants