Separate *args from **kwargs #32
Conversation
JacobEvelyn
force-pushed
the
silencing-args-warning
branch
from
2fcd2ca to
93b8c67
Compare
JacobEvelyn
force-pushed
the
silencing-args-warning
branch
from
96eb816 to
4765542
Compare
|
@jemmaissroff I hope I didn't step on your toes in finishing this out—I think when we last left off we said I'd update the benchmarks in the README but to do that I realized I needed to add a few more benchmarks and then it was easy to just also add the RSpec tests as well. This is ready for review! |
| @@ -70,6 +70,26 @@ def keyword_args(a:, b:) | |||
| 100 | |||
| end | |||
| #{benchmark_gem.memoization_method} :keyword_args | |||
|
|
|||
| def positional_and_keyword_args(a, b:) | |||
| 100 | |||
There was a problem hiding this comment.
Do we have an issue to fill these in?
There was a problem hiding this comment.
What would we fill these in with? I don't think the result would impact the benchmark since we're only benchmarking cache value returns, right?
There was a problem hiding this comment.
Ah yup you're totally right, PR lgtm. Can merge this if you approve!
Also, looks like I lost permission to push to this repo. Any chance you could check on that?
There was a problem hiding this comment.
Whoops, sorry! Should be fixed now!
This commit separates *args from **kwargs in the memo_wise'd method signatures, and only uses the minimum necessary. This allows us to gain performance benefits (fewer allocations in many cases) and silence the Ruby warnings.
JacobEvelyn
force-pushed
the
silencing-args-warning
branch
from
c593c36 to
c546911
Compare
This commit separates *args from **kwargs in the memo_wise'd
method signatures, and only uses the minimum necessary. This
allows us to gain performance benefits (fewer allocations in
many cases) and silence the Ruby warnings.
Before merging:
README.mdand update this PR