Skip to content

chore: simplify, modernize (Go 1.26), update deps#124

Open
rustatian wants to merge 1 commit into
masterfrom
chore/cleanup-modernize-deps
Open

chore: simplify, modernize (Go 1.26), update deps#124
rustatian wants to merge 1 commit into
masterfrom
chore/cleanup-modernize-deps

Conversation

@rustatian
Copy link
Copy Markdown
Member

Applied fixes

R (review/correctness) — 7 applied:

  • keys==nillen(keys)==0 in Has, MGet, Set, Delete (catches empty-slice callers)
  • time.Parse/client.Set errors in Set wrapped with errors.E(op, err)
  • Clear return value wrapped with errors.E(op, err) + op name added
  • Has/Delete: validate keyTrimmed and operate on keyTrimmed (was validating trimmed but passing untrimmed key to memcache)
  • Duplicate/misleading ErrCacheMiss comment in Delete removed

S (simplify) — 6 applied:

  • Dead if exist != nil guard removed in Has (unreachable after err != nil branch)
  • MGet: merged validate loop and fetch loop into single pass
  • Delete: collapsed two-pass validate+delete loops into one
  • Flags: 0 zero-value field omitted from memcache.Item literal
  • Inaccurate // unsafe convert comment removed from Value: field
  • if s.Addr == nillen(s.Addr) == 0 in config.go InitDefaults

M (modernize Go 1.26) — 4 applied:

  • All for i := range keys { keys[i] }for _, key := range keys (Go 1.22 range-over-value)
  • MGet: replaced N×Get loop with single GetMulti(keys) call (otelmemcache exposes it cleanly)
  • Inline one-use keyTrimmed variable in Get
  • Rename stderr "errors" alias: stdlib errors used directly, RR errors package aliased as rrerrors

Not applied (needs manual review)

  • plugin.go:50 — p.tracer always set → nil-guard in driver unreachable: the nil-guard in NewMemcachedDriver is a defensive pattern worth keeping; it's not a correctness issue.

Verification

go build ./...   ✓
go vet ./...     ✓
golangci-lint run → 0 issues ✓

- Replace `stderr "errors"` alias with stdlib `errors` + `rrerrors` alias for RR errors
- Guard all variadic params with `len(keys)==0` instead of `keys==nil`
- Use trimmed key consistently in Has/Delete (validate and operate on same value)
- Wrap errors in Set/Clear with `errors.E(op, err)`
- Merge MGet validate+iterate loops into single pass; use GetMulti single call
- Collapse Delete two-loop validate+delete into one loop
- Drop zero-value `Flags:0` field and inaccurate `// unsafe convert` comment
- Remove dead `if exist != nil` guards (unreachable after err branch)
- Switch all `for i := range` index loops to range-over-value (Go 1.22)
- Inline one-use `keyTrimmed` in Get
- Fix `if s.Addr==nil` → `len(s.Addr)==0` in InitDefaults
- Fix duplicate/misleading ErrCacheMiss comment in Delete
Copilot AI review requested due to automatic review settings May 29, 2026 12:30
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Warning

Review limit reached

@rustatian, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 45 minutes and 20 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 949e9e8c-92d8-4851-b2b4-41b7402cd3bb

📥 Commits

Reviewing files that changed from the base of the PR and between 7040e08 and d71a6de.

📒 Files selected for processing (2)
  • memcachedkv/config.go
  • memcachedkv/driver.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/cleanup-modernize-deps

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Simplifies and modernizes the memcached KV driver: aliases the RoadRunner errors package as rrerrors so the stdlib errors can be used directly for errors.Is, switches variadic-empty checks to len(...) == 0, collapses duplicate validate/fetch loops, uses GetMulti in MGet, ensures Has/Delete operate on the trimmed key, and wraps previously bare returns in Set/Clear with the op-tagged error.

Changes:

  • Rename errors import to rrerrors and use stdlib errors.Is for memcache.ErrCacheMiss checks.
  • Tighten/simplify Has, MGet, Set, Delete, Clear: nil→len checks, single-pass loops, GetMulti, consistent rrerrors.E(op, err) wrapping, range-over-value loops.
  • Config.InitDefaults now defaults when Addr is nil or empty (len(s.Addr) == 0).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
memcachedkv/driver.go Errors-package alias, simplified loops/checks, GetMulti, consistent error wrapping, trimmed-key usage in Has/Delete.
memcachedkv/config.go Default-address check now covers empty slices as well as nil.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.00%. Comparing base (7040e08) to head (d71a6de).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   65.00%   65.00%           
=======================================
  Files           1        1           
  Lines          20       20           
=======================================
  Hits           13       13           
  Misses          4        4           
  Partials        3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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