[WIP] Change MemoWise public API #247
Conversation
|
Should I wait for this to be finished before migrating to this gem? |
I am not a maintainer of the gem, so I am not sure if this will ever get merged and released. But, my aim is to keep the public API surface change to only the change from |
|
Hi @PikachuEXE, my understanding is that this is intended to be a proposal to be discussed -- which is awesome! -- but at this time, there's no current understanding nor expectation that this proposed API change will be accepted, is there? UPDATE @paracycle I see your response first, and 👍🏻 to how you expressed it! |
|
OK I get this is a proposal now I would ask
|
|
@PikachuEXE all of those points will be addressed in the PR description in detail once the build gets green on this branch. |
|
Yes! That's why the PR is marked [WIP] and is set as Draft. |
Previous versions of `MemoWise` used an array as the internal cache data structure, where each memoized method uses a unique array index. However, complex inheritance structures make it easy to have multiple methods accidentally share the same array index on the same object, resulting in nondeterministic and incorrect behavior. While we may explore that optimization again in the future, for the time being we are switching to a hash data structure keyed on method name, which is safer because two methods cannot share the same name in Ruby.
Co-authored-by: Jemma Issroff <[email protected]>
Co-authored-by: Jemma Issroff <[email protected]>
Co-authored-by: Jemma Issroff <[email protected]>
Co-authored-by: Jemma Issroff <[email protected]>
Co-authored-by: Jemma Issroff <[email protected]>
This should allow to do chaining like: ```ruby private memo_wise def foo 42 end ```
jemmaissroff
force-pushed
the
uk-ji-refactor
branch
from
de1168c
to
6d979c1
Compare
Codecov Report
@@ Coverage Diff @@
## main #247 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 206 151 -55
=========================================
- Hits 206 151 -55
Continue to review full report at Codecov.
|
|
Closed and replaced by #253 |
Re-attempt at #213 with @jemmaissroff, still a WIP...
Before merging:
README.mdand update this PRCHANGELOG.md, add an entry following Keep a Changelog guidelines with semantic versioning