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

refactor: optimize cache code architeture #2921

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

causeThenEffect
Copy link

I think refactored code can reduce code redundancy

@hazendaz
Copy link
Member

seems mostly good but what about the hash, equals, etc you removed but did not add to the main object to process with the delegate. That may in fact be important especially to the cache. Not sure if the downstream cache projects are in fact calling those. They could be so I think you need to add them just to be safe since this is a fairly big change.

@hazendaz hazendaz requested a review from harawata August 19, 2023 02:31
@hazendaz hazendaz self-assigned this Aug 19, 2023
@hazendaz hazendaz self-requested a review August 19, 2023 02:31
@harawata
Copy link
Member

Thank you for the PR, @causeThenEffect !

I haven't looked into it, but there are test failures.

@wuwen5
Copy link
Contributor

wuwen5 commented Dec 29, 2023

This appears to be a duplicate of #838. If necessary, I can reopen it.

After rebase, it's here wuwen5@a795b01

@hazendaz
Copy link
Member

hazendaz commented Jan 2, 2024

@wuwen5 Go ahead and reopen. If it is just a rehash, no one said anything the first time through, we should be able to compare them. Main thing is it has to build successfully and address any concerns.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants