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

Remove dependency on go-cmp #19

Closed
asmaloney opened this issue Apr 10, 2023 · 3 comments · Fixed by #21
Closed

Remove dependency on go-cmp #19

asmaloney opened this issue Apr 10, 2023 · 3 comments · Fixed by #21
Labels
enhancement New feature or request

Comments

@asmaloney
Copy link

Because this is a library, users have to carry around all dependencies, so the fewer the better.

I did a cursory look - is go-cmp only used in tests?

Can you get away with regular comparisons and/or using reflect.DeepEqual or is there a real need for something different?

@erni27
Copy link
Owner

erni27 commented Apr 10, 2023

No real need to use it. I've been using imcache internally in projects already having go-cmp dependency so I've missed it. Thanks!

@erni27 erni27 changed the title Suggestion: Remove dependency on go-cmp Remove dependency on go-cmp Apr 10, 2023
@erni27 erni27 added the enhancement New feature or request label Apr 10, 2023
@asmaloney
Copy link
Author

👍 "No dependencies" is a good feature!

(If I can sneak in another thing 😄 - it might be useful to compare imcache vs. other similar libraries in the README? I'm thinking features/benefits/drawbacks, not benchmarks.)

Thank you for taking the time to put this together!

@erni27
Copy link
Owner

erni27 commented Apr 10, 2023

Yes, I agree. It would be useful. I believe users looking for an in-memory cache library can be unaware of the differences between them just by looking at their APIs so would be great to be explicit about it.

EDIT: I've created a new issue for that. #20

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

Successfully merging a pull request may close this issue.

2 participants