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 43 minutes and 10 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 ignored due to path filters (1)
📒 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().
Two bare same-typed string returns (scheme, addr) are positionally ambiguous at call sites; a small struct makes each field self-documenting and swap-proof. Validation semantics (reject multi-"://") unchanged.
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
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