trailblazer / cells

View components for Ruby and Rails.
https://trailblazer.to/2.1/docs/cells.html
3.06k stars 236 forks source link

Decrease number of deprecation warnings in Ruby 2.7 #496

Closed richardboehme closed 2 weeks ago

richardboehme commented 2 years ago

We experienced some deprecation warnings after upgrading one of our apps to Ruby 2.7.5. This patch removes all deprecation warnings occurring when running the tests of the cells gem.

apotonick commented 2 years ago

Overally, I'm very happy you address this as I've been trying to make Cells ready for Ruby 3.0. :sun_with_face: I see a problem, however, in our old signatures: we were allowing all kinds of argument chains such, and maybe a first step would be to define expected arguments. For example, the #cell helper should probably be def cell(model=nil, **options) and not allow positionals.

Are you happy to discuss this approach?

richardboehme commented 2 years ago

Overally, I'm very happy you address this as I've been trying to make Cells ready for Ruby 3.0. sun_with_face I see a problem, however, in our old signatures: we were allowing all kinds of argument chains such, and maybe a first step would be to define expected arguments. For example, the #cell helper should probably be def cell(model=nil, **options) and not allow positionals.

Are you happy to discuss this approach?

That sounds like a way better approach than what I was going for! I just wanted to quickly fix those warnings :sweat_smile: Having defined expected arguments is definitely better than what we have right now. How do you want to approach this refactoring? I feel like it might lead to breaking other cells-related gems (I'm thinking trailblazer/cells-rails for example). Also it might break some user's cells invocation when they pass the options as a positional right now (however this would be an easy fix for them).

apotonick commented 2 years ago

Komm mal hier her https://trailblazer.zulipchat.com @richardboehme :beers: