Conversation
WalkthroughThis PR updates dependency versions in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
go.mod (1)
3-5: Remove leftover errorutil calls and update CI to Go 1.24 toolchain
- pkg/providers/gcp/gcp.go & pkg/providers/gcp/auth.go still reference
errorutil.New/NewWithErr—replace or remove these usages.- No
.github/workflowsfound; ensure your CI (GitHub Actions, CircleCI, etc.) installs Go 1.24 or respects thetoolchain go1.24.2directive in go.mod.
🧹 Nitpick comments (6)
pkg/providers/gcp/assets_api.go (4)
435-446: Include the offending asset name in errors for faster triageUse errkit.Newf to embed assetName in the error for on-call debugging.
- return "", "", "", errkit.New("invalid compute asset name format") + return "", "", "", errkit.Newf("invalid compute asset name format: %q", assetName) @@ - return "", "", "", errkit.New("unexpected compute asset name format") + return "", "", "", errkit.Newf("unexpected compute asset name format: %q", assetName)
451-463: Do the same for Cloud Functions parse errorsEmbed the assetName.
- return "", "", "", errkit.New("invalid function asset name format") + return "", "", "", errkit.Newf("invalid function asset name format: %q", assetName) @@ - return "", "", "", errkit.New("unexpected function asset name format") + return "", "", "", errkit.Newf("unexpected function asset name format: %q", assetName)
468-480: Do the same for Cloud Run parse errorsEmbed the assetName.
- return "", "", "", errkit.New("invalid cloud run asset name format") + return "", "", "", errkit.Newf("invalid cloud run asset name format: %q", assetName) @@ - return "", "", "", errkit.New("unexpected cloud run asset name format") + return "", "", "", errkit.Newf("unexpected cloud run asset name format: %q", assetName)
485-504: Do the same for DNS parse errorsEmbed the assetName on the error path.
- return "", "", "", "", errkit.New("invalid DNS asset name format") + return "", "", "", "", errkit.Newf("invalid DNS asset name format: %q", assetName) @@ - return "", "", "", "", errkit.New("unexpected DNS asset name format") + return "", "", "", "", errkit.Newf("unexpected DNS asset name format: %q", assetName)pkg/providers/k8s/kubernetes.go (2)
39-41: Fix message typo (double space)Minor text nit.
- return nil, errkit.New("no kubeconfig_file or kubeconfig_encoded provided") + return nil, errkit.New("no kubeconfig_file or kubeconfig_encoded provided")
168-171: Use errkit.Wrap instead of fmt.Errorf for consistencyAligns with the rest of the file and keeps error chains uniform.
- kubeConfig, err := modifiedClientConfig.ClientConfig() - if err != nil { - return nil, fmt.Errorf("could not get client config: %v", err) - } + kubeConfig, err := modifiedClientConfig.ClientConfig() + if err != nil { + return nil, errkit.Wrap(err, "could not get client config") + }After this change, fmt becomes unused; please drop it from imports (goimports will handle):
import ( "context" "encoding/base64" - "fmt" "strings"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.mod(4 hunks)pkg/providers/dnssimple/dnssimple.go(2 hunks)pkg/providers/gcp/assets_api.go(6 hunks)pkg/providers/k8s/kubernetes.go(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
pkg/providers/gcp/assets_api.go (3)
pkg/providers/dnssimple/dnssimple.go (1)
New(45-94)pkg/providers/k8s/kubernetes.go (1)
New(33-89)pkg/providers/gcp/gcp.go (1)
New(182-199)
pkg/providers/dnssimple/dnssimple.go (1)
pkg/providers/k8s/kubernetes.go (1)
New(33-89)
pkg/providers/k8s/kubernetes.go (2)
pkg/providers/dnssimple/dnssimple.go (1)
New(45-94)pkg/providers/gcp/gcp.go (1)
New(182-199)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test Builds (1.22.x, macOS-latest)
- GitHub Check: Test Builds (1.22.x, ubuntu-latest)
- GitHub Check: Test Builds (1.22.x, windows-latest)
- GitHub Check: Lint Test
- GitHub Check: Analyze (go)
- GitHub Check: release-test
🔇 Additional comments (5)
pkg/providers/gcp/assets_api.go (1)
11-11: errkit import — LGTMConsistent with the repo-wide migration.
pkg/providers/k8s/kubernetes.go (2)
10-10: errkit import — LGTM
49-54: Error wrapping migration — LGTMerrkit.Wrap usage is consistent and preserves context.
Also applies to: 58-65, 111-112, 122-123, 135-136, 150-155
pkg/providers/dnssimple/dnssimple.go (1)
83-87: errkit.Wrap/New migration — LGTMError context is preserved and messages remain clear.
go.mod (1)
32-34: Dependency bumps — LGTMUpdates look safe; utils v0.5.0 aligns with errkit usage; xz v0.5.14 addresses prior CVEs.
Also applies to: 146-146, 183-185, 209-209, 217-220
bump deps, fix lint errs
closes #702
Summary by CodeRabbit