codenoble / cache-crispies

Speedy Rails JSON serialization with built-in caching
MIT License
156 stars 16 forks source link

Splat keyword arguments correctly when initializing a plan for collection #42

Closed Flixt closed 2 years ago

Flixt commented 2 years ago

With Ruby 3.1.0 we encountered the following error when enabling caching for collections.

ArgumentError:
       wrong number of arguments (given 3, expected 2)
     # ./lib/cache_crispies/plan.rb:19:in `initialize'
     # ./lib/cache_crispies/collection.rb:44:in `new'
     # ./lib/cache_crispies/collection.rb:44:in `block in cached_json'
     # ./lib/cache_crispies/collection.rb:43:in `each'
     # ./lib/cache_crispies/collection.rb:43:in `each_with_object'
     # ./lib/cache_crispies/collection.rb:43:in `cached_json'
     # ./lib/cache_crispies/collection.rb:26:in `as_json'

Because the Collection does currently not splat the options hash to keyword arguments. I could not come up with a good spec for this case, as the specs for the Collection class mock the initialization of the plan itself. If you have a good idea on how to write a spec to avoid a regression I would be happy to add it.

Additional question: What do you think about adding different Gemfiles for different ruby versions?

adamcrown commented 2 years ago

Thanks @Flixt . This looks good to me. I think I'm okay not having a spec for this, at least for now.

I'd be okay having different Gemfiles for different ruby versions. We've already got a few different Gemfiles for the Rails versions. But honestly, it's probably time to stop supporting some of the 5.x versions. So we I'd be happy to cut back there.