feat: build quic addresses - #177
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: getoptimum/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds QUIC advertised-address builders using UDP and Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant BuildAdvertisedQUICAddresses
participant GetInterfaceIPs
participant slices.UniqueSlice
Caller->>BuildAdvertisedQUICAddresses: provide public IPs and listen port
BuildAdvertisedQUICAddresses->>GetInterfaceIPs: enumerate interface IPv4 addresses
GetInterfaceIPs-->>BuildAdvertisedQUICAddresses: return interface addresses
BuildAdvertisedQUICAddresses->>slices.UniqueSlice: deduplicate QUIC multiaddresses
slices.UniqueSlice-->>BuildAdvertisedQUICAddresses: return unique addresses
BuildAdvertisedQUICAddresses-->>Caller: return addresses or error
Suggested reviewers: 🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (7 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/net/addr_advertise_quic_test.go`:
- Around line 3-9: Update the import block in the test file to match the
repository’s gci grouping and ordering rules, keeping standard-library imports
separated from third-party imports and preserving all existing dependencies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 200da61f-d8da-467d-aeff-0e656e5c7b13
📒 Files selected for processing (2)
pkg/net/addr_advertise.gopkg/net/addr_advertise_quic_test.go
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/getoptimum/optimum-common/pkg/logger" | ||
| "github.com/getoptimum/optimum-common/pkg/net" | ||
| "github.com/stretchr/testify/require" | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Apply the repository’s gci import grouping.
The lint job fails on this import block, blocking CI.
Proposed fix
import (
"testing"
"github.com/getoptimum/optimum-common/pkg/logger"
"github.com/getoptimum/optimum-common/pkg/net"
+
"github.com/stretchr/testify/require"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import ( | |
| "testing" | |
| "github.com/getoptimum/optimum-common/pkg/logger" | |
| "github.com/getoptimum/optimum-common/pkg/net" | |
| "github.com/stretchr/testify/require" | |
| ) | |
| import ( | |
| "testing" | |
| "github.com/getoptimum/optimum-common/pkg/logger" | |
| "github.com/getoptimum/optimum-common/pkg/net" | |
| "github.com/stretchr/testify/require" | |
| ) |
🧰 Tools
🪛 GitHub Actions: Lint / golangci _ golangci-lint
[error] 5-5: golangci-lint reported formatting issue (gci): File is not properly formatted (gci).
🪛 GitHub Check: golangci / golangci-lint
[failure] 5-5:
File is not properly formatted (gci)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/net/addr_advertise_quic_test.go` around lines 3 - 9, Update the import
block in the test file to match the repository’s gci grouping and ordering
rules, keeping standard-library imports separated from third-party imports and
preserving all existing dependencies.
Sources: Linters/SAST tools, Pipeline failures
There was a problem hiding this comment.
Pull request overview
Adds helpers in pkg/net to construct advertised QUIC-v1 multiaddrs (IPv4 required, IPv6 optional) alongside existing TCP advertised address builders, with unit tests mirroring the TCP coverage.
Changes:
- Introduces
BuildAdvertisedQUICAddressesandMustBuildAdvertisedQUICAddressesto generate/ip{4,6}/.../udp/<port>/quic-v1multiaddrs. - Extends address collection to include the configured public IPs plus interface IPv4 addresses, with de-duplication.
- Adds a dedicated test file validating valid inputs, invalid IPs, and port bounds.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/net/addr_advertise.go | Adds QUIC advertised address builders (including a Must* wrapper) mirroring the existing TCP builder behavior. |
| pkg/net/addr_advertise_quic_test.go | Adds unit tests for QUIC advertised address building, paralleling existing TCP tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary by CodeRabbit