-
Notifications
You must be signed in to change notification settings - Fork 602
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
Go defaults to "0"" so in case we want to return #181
base: main
Are you sure you want to change the base?
Conversation
EntryStatus back to the caller "Expired" cannot be differentiated. Fixing this by default "_" to 0 and incremental RemoveReasons comment was missing for GetWithInfo api so updated that test: ran all unit test cases
I quite like this PR and problem it solves, but I agree this is definitely a 3.0 decision since it could break a lot of downstream configurations when they upgrade. This should definitely go in the release notes. |
@mxplusb this PR can be open until its ready to go in. I kept it here so that I do not forget about this :) |
Great, thanks! |
So, the error |
@flisky good point, we can remove it. But anyway, this PR is for a next major version, so it will be safe to make a breaking change. |
@jgheewala when you get a chance, can you rebase? It would be good to keep this updated to prep for the next major version. |
May I take over of it, @jgheewala & @mxplusb ? Besides these changes, I also want to return the expired entry even when |
@flisky let's create a separate issue for your idea and discuss it there. This will make discussion much more visible and structured, thanks :) |
@mxplusb was super busy will rebase my changes soon |
@mxplusb done with the conflicts. |
@jgheewala there's a new branch, called |
Codecov Report
@@ Coverage Diff @@
## master #181 +/- ##
==========================================
+ Coverage 86.56% 87.01% +0.45%
==========================================
Files 15 15
Lines 640 647 +7
==========================================
+ Hits 554 563 +9
+ Misses 71 68 -3
- Partials 15 16 +1
Continue to review full report at Codecov.
|
Currently all const for RemovedReason are explicitly set to avoid any breaking changes. For v3 migrating the reasons to iota makes code readability and managing addition of more reasons easier in the future. Test: Validated all test cases run successfully.
Codecov Report
@@ Coverage Diff @@
## master #181 +/- ##
==========================================
+ Coverage 86.56% 87.01% +0.45%
==========================================
Files 15 15
Lines 640 647 +7
==========================================
+ Hits 554 563 +9
+ Misses 71 68 -3
- Partials 15 16 +1
Continue to review full report at Codecov.
|
EntryStatus back to the caller "Expired" cannot be differentiated.
Fixing this by default "_" to 0 and incremental RemoveReasons
comment was missing for GetWithInfo api so updated that
test: ran all unit test cases