solidusio / solidus_subscriptions

An extension to add subscriptions to your Solidus store.
BSD 3-Clause "New" or "Revised" License
48 stars 53 forks source link

Don't allocate an empty hash for each call to #eligible? #286

Closed elia closed 1 year ago

elia commented 1 year ago

Summary

This error was reported:

ArgumentError (wrong number of arguments (given 2, expected 1)):
solidus_subscriptions (5b4c7052ace3) app/models/solidus_subscriptions/promotion/rules/subscription_creation_order.rb:27:in `eligible?'

And it looks like solidus is not using keyword arguments inside #eligible? so we'll stop doing it here as well, probably wasn't a problem until the ruby version was bumped.

Fixed by #283

Also set the default value to nil so no additional hashes are allocated.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

kennyadsl commented 1 year ago

@elia duplicate of #283 ?

elia commented 1 year ago

@kennyadsl yes, rebasing and renaming to just avoid the extra allocation πŸ‘πŸ‘πŸ‘

kennyadsl commented 1 year ago

@nirebu since this is not covered by specs here, do you have any chance to test if this change will work?

elia commented 1 year ago

Closing, as the impact of this is negligible as the method is generally called forwarding the options arg.