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

Proposal: Deprecate let! #1951

Closed
andyw8 opened this issue Apr 28, 2015 · 4 comments
Closed

Proposal: Deprecate let! #1951

andyw8 opened this issue Apr 28, 2015 · 4 comments

Comments

@andyw8
Copy link
Contributor

andyw8 commented Apr 28, 2015

let! seems to be one of the most often mis-used parts of RSpec's syntax.

The documentation says that let is for a memorised helper method, but very often I see it being used for setting up state, where a before block would be much more intention-revealing.

For the occasional cases where there is a need to store a reference to the result of the before block, a second argument could be passed, for example:

before(:each, :widgets_category) { Category.create!(name: "widgets") }

Thoughts?

@JonRowe
Copy link
Member

JonRowe commented Apr 28, 2015

let! seems to be one of the most often mis-used parts of RSpec's syntax.

I don't see that as a misuse, let is designed to give names to your setup, often you don't need them straight away but sometimes it's nicer to use let! than before { my_let }, I guess I consider an alternate name but personally I'm ok with let!.

before(:each, :widgets_category) { Category.create!(name: "widgets") }

Additionally we can't do this because before already takes additional arguments (metadata filters)

@yujinakayama
Copy link
Member

I agree with @JonRowe. IMO let! is reasonable name since it represents side effects with ! and also is easy to be changed to let by removing 1 character.

@myronmarston
Copy link
Member

Some users like let!; some don't. Those who don't like it can easily disable it in their projects:

module DisableLetBang
  def let!(*args)
    raise "let! is not allowed"
  end
end

RSpec.configure do |c|
  c.extend DisableLetBang
end

I don't think it makes sense for us to deprecate it and remove it, as that would create unnecessary work for those who like it and have been using it. Closing.

@andyw8
Copy link
Contributor Author

andyw8 commented Apr 28, 2015

Thanks for the feedback!

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

No branches or pull requests

4 participants