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

Migrate v2 to using v3 code instead of duplicating it #11

Closed
wants to merge 3 commits into from

Conversation

paskal
Copy link
Collaborator

@paskal paskal commented Feb 20, 2024

Based on #10. Instead of duplicating code, which would be the same in v3, this PR changes v2 to utilize v3, keeping things the same for the user while simplifying code maintenance.

Copy link

github-actions bot commented Feb 20, 2024

Pull Request Test Coverage Report for Build 7976974605

Details

  • -11 of 225 (95.11%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-4.9%) to 95.111%

Changes Missing Coverage Covered Lines Changed/Added Lines %
v3/cache.go 198 209 94.74%
Totals Coverage Status
Change from base Build 7953908422: -4.9%
Covered Lines: 214
Relevant Lines: 225

💛 - Coveralls

@paskal paskal force-pushed the paskal/v2_by_v3 branch 2 times, most recently from 045ee52 to 1025e35 Compare February 20, 2024 16:44
@paskal paskal force-pushed the paskal/v2_by_v3 branch 2 times, most recently from ba717b7 to 5003f50 Compare February 20, 2024 19:57
v2 had most of the required functions, so this change adds missing ones
to satisfy the simplelru interface.

To do that, RemoveOldest started returning parameters, unlike being void
as before.

Another behaviour change is that Get and Peek now return
the cached value in case it has already expired to be consistent with
the `simplelru` implementation.

Also, GetExpiration is added.
"time"

v3 "github.com/go-pkgz/expirable-cache/v3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good idea. First of all, such a dependency feels unnatural and confusing; second - it will bring the v3 package as well as one wanted to include v2. And with vendored deps it will be even more confusing. My vote is to keep them separate

@paskal paskal closed this Feb 20, 2024
@paskal paskal deleted the paskal/v2_by_v3 branch February 20, 2024 20:31
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.

2 participants