-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Rewrite internal cache package for lcw requirements #17
Conversation
if !ok { | ||
c.data[key] = &cacheItem{} | ||
} | ||
c.data[key].data = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this assignment can be potentially risky if the value has some sort of mutable elements in, like a slice. Not sure if we should care about ti and perform some deep copy. Probably makes sense to see what other caches do in this case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go-cache does nothing, gcache provides ability to set serializeFunc but it's not provided out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ristretto does nothing as well. Seems to be up to a caller not to shoot himself in a leg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, let's keep it as-is, but mention it in the readme as something user should be aware of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#18 created to fix it.
Simplify the logic, removing functions calls and storage. Stats removed as they are provided by lcw itself.