Skip to content

improve clarity when member is not exist #1943

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

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from

Conversation

chx9
Copy link
Contributor

@chx9 chx9 commented Apr 11, 2025

this pr aims to fix one of the issues from Redis Triage
redis/redis#13350
it has been more than 1 year since this issue brought up

@chx9 chx9 force-pushed the improve-geo-clarity branch from 02c6fe8 to a96996a Compare April 11, 2025 12:38
Copy link

codecov bot commented Apr 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.93%. Comparing base (44dafba) to head (a96996a).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1943      +/-   ##
============================================
- Coverage     70.99%   70.93%   -0.06%     
============================================
  Files           123      123              
  Lines         65769    65770       +1     
============================================
- Hits          46693    46656      -37     
- Misses        19076    19114      +38     
Files with missing lines Coverage Δ
src/geo.c 93.59% <100.00%> (+0.01%) ⬆️

... and 15 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Fusl
Copy link
Contributor

Fusl commented Apr 14, 2025

longLatFromMember can also return C_ERR if geohashDecodeToLongLatType returns a decoding error while the member did exist. Should we maybe differentiate those cases or just return member %s does not exist or failed to decode?

@madolson
Copy link
Member

longLatFromMember can also return C_ERR if geohashDecodeToLongLatType returns a decoding error while the member did exist. Should we maybe differentiate those cases or just return member %s does not exist or failed to decode?

Ideally yes. It would be nice to differentiate the member be missing from some type of corruption.

@chx9 chx9 force-pushed the improve-geo-clarity branch from 2be4698 to 41fa823 Compare April 15, 2025 12:38
@chx9
Copy link
Contributor Author

chx9 commented Apr 15, 2025

longLatFromMember can also return C_ERR if geohashDecodeToLongLatType returns a decoding error while the member did exist. Should we maybe differentiate those cases or just return member %s does not exist or failed to decode?

Ideally yes. It would be nice to differentiate the member be missing from some type of corruption.

agree, i added error infomation for each case.

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.

3 participants