chore(dev): add go.work for gopls multi-module workspace (closes #516)#858
chore(dev): add go.work for gopls multi-module workspace (closes #516)#858cristim wants to merge 6 commits into
Conversation
cristim
added
triaged
|
@coderabbitai review |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughGo workspace local files are excluded via gitignore, with contributor guidance added for multi-module development. Test execution is configured to disable AWS EC2 metadata to prevent retry delays. Dependencies across the root and all provider modules are updated to newer AWS SDK v2, OpenTelemetry, and Google Cloud versions. A context cancellation guard is added to GCP recommendations to prevent error swallowing. ChangesGo Workspace Setup, Dependency Maintenance, Test Configuration, and Context Cancellation Fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
112-114: ⚡ Quick winClarify gopls discovery of
go.work.local.Option B suggests that gopls will "auto-discover"
go.work.local, but this is misleading. By default, gopls searches forgo.workby walking up the directory tree. It does not automatically discovergo.work.localunlessGOWORKis explicitly set to point to it.The earlier text (line 106) mentions "symlinking it over
go.worktemporarily" as an alternative, but Option B doesn't demonstrate this approach. Consider either:
- Removing Option B and keeping only Option A (setting
GOWORK)- Rewriting Option B to demonstrate the symlink approach:
ln -sf go.work.local go.work🤖 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 `@CONTRIBUTING.md` around lines 112 - 114, Option B currently misstates gopls behavior for go.work.local; update the Option B text to either remove it (leaving only the GOWORK approach in Option A) or rewrite it to show the symlink approach so gopls sees the workfile (e.g., describe creating a symlink from go.work.local to go.work using a command like ln -sf go.work.local go.work), and explicitly note that gopls only auto-discovers go.work by walking up directories unless GOWORK is set to point to go.work.local.
🤖 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 `@CONTRIBUTING.md`:
- Around line 119-121: The current regen command uses word-splitting-prone
substitution: go work edit -use $(git worktree list --porcelain | awk
'/^worktree/{print $2}' | tail -n +2 | tr '\n' ' '), which fails on worktree
paths with spaces/special chars; change the pipeline to emit and consume
null-delimited paths (e.g., git worktree list --porcelain | awk
'/^worktree/{print $2 "\\0"}' or use git worktree list -z) and pass them safely
to go work edit via xargs -0 or a while IFS= read -r -d '' loop so paths aren’t
split, and update the verification snippet/test data to include at least one
worktree path containing a space (e.g., "work with space") to cover the failure
mode.
In `@go.mod`:
- Line 9: The go.mod currently pins github.com/aws/aws-sdk-go-v2/config but the
review asks you to verify module versions and run the same security checks for
OpenTelemetry and Google Cloud Storage; confirm the exact versions for
github.com/aws/aws-sdk-go-v2/config, github.com/aws/aws-sdk-go-v2/credentials,
github.com/aws/aws-sdk-go-v2/service/sts,
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc
(otelgrpc) and cloud.google.com/go/storage are correct and present in go.mod,
then run a Go vulnerability scan (e.g., govulncheck or go list -m -u -json all
plus GitHub advisory queries) against those modules and update go.mod to bump or
replace any vulnerable versions you find; reference the module names
(github.com/aws/aws-sdk-go-v2/config, otelgrpc, cloud.google.com/go/storage)
when making changes so reviewers can re-run the security check.
---
Nitpick comments:
In `@CONTRIBUTING.md`:
- Around line 112-114: Option B currently misstates gopls behavior for
go.work.local; update the Option B text to either remove it (leaving only the
GOWORK approach in Option A) or rewrite it to show the symlink approach so gopls
sees the workfile (e.g., describe creating a symlink from go.work.local to
go.work using a command like ln -sf go.work.local go.work), and explicitly note
that gopls only auto-discovers go.work by walking up directories unless GOWORK
is set to point to go.work.local.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: eaae2872-d6f1-4217-81ac-248e75f63743
⛔ Files ignored due to path filters (7)
go.sumis excluded by!**/*.sumgo.workis excluded by!**/*.workgo.work.sumis excluded by!**/*.sumpkg/go.sumis excluded by!**/*.sumproviders/aws/go.sumis excluded by!**/*.sumproviders/azure/go.sumis excluded by!**/*.sumproviders/gcp/go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.gitignoreCONTRIBUTING.mdgo.modpkg/go.modproviders/aws/go.modproviders/azure/go.modproviders/gcp/go.modproviders/gcp/recommendations.go
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Commit a minimal go.work listing only the primary module so gopls resolves imports for standard clones without any local config. Per-developer worktree paths go in a gitignored go.work.local file; the pattern and a quick-regeneration snippet are documented in CONTRIBUTING.md under the new "Go workspace and worktrees" section. .gitignore is updated to track go.work and go.work.sum (committed) while ignoring go.work.local and go.work.local.sum (developer-local).
…lings (refs #516) The initial go.work only listed the root module, leaving the sibling modules unresolved by gopls. Add all six modules (root, pkg, providers/aws, providers/azure, providers/gcp, tests/e2e) to the use block, run go work sync to align module go.mod files with the workspace-selected versions, and go mod tidy in each module. Update the CONTRIBUTING.md workspace section to match the multi-module go.work. All modules verified: go build ./... succeeds for each, go vet ./... from root is clean, and tests/e2e passes go vet -tags=e2e.
Aligning providers/gcp deps with the workspace (cloud.google.com/go/compute v1.23.3 -> v1.54.0, the version the root binary already builds against) surfaced a latent gap: the newer REST client can complete the regions List call and return a real 403 even when the caller's ctx is already cancelled. That 403 was then swallowed by the permission-warning branch, returning (nil-error, empty recs) instead of context.Canceled and breaking the contract pinned by TestRecommendationsClientAdapter_GetRecommendations_PropagatesContextCancellation. Guard GetRecommendations with an explicit ctx.Err() check before any API fan-out so cancellation propagates deterministically and no network call is attempted on an already-dead context.
The documented regeneration one-liner used an unquoted command
substitution plus awk '{print $2}', which truncates worktree paths at
the first space and word-splits the result before passing it to
go work edit -use. Replace it with a sed extraction that keeps the
full path and a while IFS= read -r loop that adds one -use entry per
worktree, verified against a worktree path containing a space.
1 participant
Summary
go.worklisting only the primary module (.) so gopls resolves imports for all standard clones with no local config needed.go.work.local(gitignored) per-developer pattern in CONTRIBUTING.md for developers working across multiple git worktrees simultaneously; includes a quick-regeneration snippet usinggit worktree list..gitignore: removes the formergo.work/go.work.sumentries (they are now committed), addsgo.work.local/go.work.local.sum(developer-local, gitignored).Test plan
go build ./...passes in a fresh clone with only the committedgo.workpresent.go.work.localwith an additionalusepath and settingGOWORK=$PWD/go.work.localcauses gopls to resolve imports in that sibling worktree without errors.go.work.localdoes not appear ingit status(correctly gitignored).go.workandgo.work.sumdo appear ingit statusas tracked (no longer gitignored).Summary by CodeRabbit
.gitignoreto ignorego.work.localartifacts.