chore: simplify, modernize (Go 1.26), update deps#108
Conversation
- config.go: extract parseDSN helper (strings.Cut) to deduplicate DSN validation between Valid() and Dialer(); replace two strings.Split+len sites with the helper - plugin.go: rename local WholeCfg → wholeCfg (unexported style); replace TrimPrefix+Index+slice with strings.Cut
|
Warning Review limit reached
More reviews will be available in 59 minutes and 8 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 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR simplifies and modernizes the RPC plugin by deduplicating DSN parsing/validation logic in Config and using newer strings.Cut-based parsing where applicable.
Changes:
- Extracted
parseDSNhelper inconfig.goand reused it inValid()andDialer(). - Renamed a local variable in
plugin.goto follow unexported naming conventions and simplified service-name extraction usingstrings.Cut.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| config.go | Adds parseDSN helper and reuses it for config validation and dialing. |
| plugin.go | Minor naming cleanup and simplifies mount-path parsing for service reflection. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #108 +/- ##
==========================================
+ Coverage 61.80% 63.94% +2.14%
==========================================
Files 3 3
Lines 144 147 +3
==========================================
+ Hits 89 94 +5
+ Misses 37 36 -1
+ Partials 18 17 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
strings.Cut only splits on the first delimiter, so the parseDSN refactor accepted inputs like tcp://host://port that the prior strings.Split(...); len != 2 check rejected. Restore exactly-one-separator semantics and add a regression test covering Valid() and Dialer().
Applied fixes
Simplify (S) — 2 applied:
config.go: extractedparseDSNhelper to deduplicate DSN validation logic shared betweenValid()andDialer()— was copy-pasted with identical error messageplugin.go: renamed local variableWholeCfg→wholeCfg(unexported naming convention)Modern Go (M) — 3 applied:
config.go:45(Valid):strings.Split(Listen, "://")+lencheck →strings.CutviaparseDSNconfig.go:66(Dialer): samestrings.Split+len→strings.CutviaparseDSNplugin.go:128:strings.TrimPrefix+strings.Index+ slice →strings.CutNot applied (needs manual review)
config.go:65—Dialer()usescontext.Background()instead of a caller-supplied context. Fixing this requires a signature change (Dialer(ctx context.Context)) and updating all call-sites; left for a dedicated change.Deps
go get -u all && go mod tidyin both.and./tests;go work sync— no version changes (all deps were already current).Verification