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

Fix memoization for optional args #75

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ashu210890
Copy link

@ashu210890 ashu210890 commented Jul 17, 2018

Fixes #76

Use a fixed :memoist_reload argument instead of depending on
argument order as it is fragile in case of optional
and named args.
This change maintains backward compatibility for methods that
don't have optional/named arguments.
@matthewrudy
Copy link
Owner

@ashu210890 any idea why this breaks all the tests?

which bit isn't backwards compatible?

@swrobel
Copy link

swrobel commented Aug 3, 2019

@ashu210890 this seems like a worthwhile fix ... are you still interested in getting it merged?

@sebjacobs sebjacobs force-pushed the master branch 2 times, most recently from 9d66c95 to 34f90dd Compare November 8, 2019 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memoization broken for optional args
3 participants