Skip to content

chore: simplify + modernize (Go 1.26)#119

Merged
rustatian merged 1 commit into
masterfrom
chore/cleanup-modernize-deps
Jun 3, 2026
Merged

chore: simplify + modernize (Go 1.26)#119
rustatian merged 1 commit into
masterfrom
chore/cleanup-modernize-deps

Conversation

@rustatian

@rustatian rustatian commented May 29, 2026

Copy link
Copy Markdown
Member

Cleanup pass from a multi-plugin review (/simplify + /use-modern-go Go 1.26 + /review).

Changes

  • Bug/dead code: resolveIP XFF branch had a dead, inverted guard if len(fwd) < s (unreachable — strings.Index already returned s >= 0). Replaced the whole Index+slice dance with strings.Cut, which returns the full string when no comma is present.
  • Modern Go: range-over-value (for _, subnet := range p.trusted) instead of index loop.
  • Dead code: removed the unused exported Cidrs type (and its now-unused net import) from config.go.
  • Hygiene: moved the test-only inc() helper into trusted_test.go; collapsed a single-entry var() block.

Deps

go get -u all (root + tests) and go work sync were no-ops — module already current.

Summary by CodeRabbit

  • Refactor

    • Simplified IP header parsing logic for improved efficiency
    • Optimized trusted subnet membership iteration
    • Removed unused exported type from package API
  • Tests

    • Added test helper for IP iteration validation

Review Change Stack

Review findings:
- resolveIP XFF: replace strings.Index + dead `len(fwd)<s` guard with strings.Cut
- Middleware: range-over-value instead of index loop over p.trusted
- drop unused exported Cidrs type (and now-unused net import) from config.go
- move test-only inc() helper into trusted_test.go
- collapse single-entry var() block

Deps already current (go get -u all + go work sync were no-ops).
Copilot AI review requested due to automatic review settings May 29, 2026 12:15
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR refactors the proxy IP parser package by removing the exported Cidrs type from the public API, relocating an internal inc() helper from plugin code to tests, simplifying IP header parsing with strings.Cut, and applying minor code-style improvements to variable declarations and loop iteration patterns.

Changes

Core Refactoring and API Cleanup

Layer / File(s) Summary
Public API cleanup
config.go
Removes the exported Cidrs type alias and net import, leaving only the Config struct as the public interface.
Helper relocation and IP parsing simplification
plugin.go, trusted_test.go
Moves the inc() helper function from plugin.go to trusted_test.go where it is used; simplifies X-Forwarded-For header parsing in resolveIP by replacing manual strings.Index and slicing logic with strings.Cut.
Code style refactoring
plugin.go
Refactors forwardedRegex from a parenthesized var (...) block to a single var assignment; converts the trusted-subnet membership loop from index-based to range-based iteration while preserving logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A helper hops away to tests where it belongs,
Parsing gets trimmer with strings that sing songs,
Vars shrink and loops flow in ranges so clean,
API unwrapped—the cleanest we've seen!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main objectives: cleanup and modernization for Go 1.26 compatibility.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description clearly explains the reasoning (cleanup pass with linting suggestions), provides comprehensive change descriptions with categorization (Bug/dead code, Modern Go, Dead code, Hygiene), and includes verification steps performed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Cleanup pass on the proxy_ip_parser RoadRunner middleware: removes dead/unreachable code, modernizes a loop, and tidies declarations. No behavior changes intended.

Changes:

  • Simplify XFF parsing in resolveIP using strings.Cut, eliminating an unreachable len(fwd) < s guard.
  • Use range-over-value in the trusted-subnet loop; collapse the single-entry var() block.
  • Remove the unused exported Cidrs type (and net import) from config.go; move the test-only inc() helper into trusted_test.go.

Reviewed changes

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

File Description
plugin.go Simplify XFF branch with strings.Cut; range-over-value loop; collapse var block; drop inc() helper.
trusted_test.go Host the test-only inc() helper now removed from plugin.go.
config.go Drop unused Cidrs type and net import.

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

@rustatian rustatian self-assigned this Jun 3, 2026
@rustatian rustatian merged commit 976af44 into master Jun 3, 2026
7 of 8 checks passed
@rustatian rustatian deleted the chore/cleanup-modernize-deps branch June 3, 2026 19:43
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