Skip to content

fix(go): ensure extraDependencies overrides take precedence over bundled deps#14587

Merged
jsklan merged 2 commits intomainfrom
devin/FER-9453-1775256515
Apr 6, 2026
Merged

fix(go): ensure extraDependencies overrides take precedence over bundled deps#14587
jsklan merged 2 commits intomainfrom
devin/FER-9453-1775256515

Conversation

@jsklan
Copy link
Copy Markdown
Contributor

@jsklan jsklan commented Apr 3, 2026

Description

Refs FER-9453

The Go v1 generator's module.imports config (the Go equivalent of extraDependencies) could not override bundled dependency versions. When a user specified module.imports, the entire default import set was replaced instead of merged, causing all unspecified defaults (uuid, testify, yaml) to be silently dropped.

The v2 generator (TypeScript) already handled this correctly via object spread ({...defaults, ...userImports}). This PR aligns v1 behavior with v2.

Changes Made

  • generators/go/internal/cmd/cmd.go: Changed moduleConfigFromCustomConfig to merge user-specified imports on top of defaultImports instead of replacing them. Default deps are copied first, then user entries are applied, so user versions override matching keys while preserving the rest.
  • generators/go/sdk/versions.yml: Added changelog entry (v1.33.1).
  • scripts/validate-all-changelogs.sh: Removed stale ruby-sdk entry that was causing the "Validate Changelogs" CI check to fail. ruby-sdk was replaced by ruby-sdk-v2 in chore(ruby): update get-test-matrix to reference ruby-sdk-v2 #14585 but this script was not updated, breaking changelog validation for all PRs touching generators/**/versions.yml.

Key Review Points

  • Behavioral change: Previously, specifying any module.imports would discard all defaults. Now defaults are always included as a base. If any user relied on the old behavior to intentionally exclude a default dep, they would now get it back. This seems like the correct trade-off — and matches how v2 already works.
  • Ranging over a nil customConfig.Module.Imports map in Go is a safe no-op, so no nil guard is needed.
  • For SDK generation (client mode), v2 overwrites go.mod anyway, so this fix primarily impacts model-only generation in v1.

Human Review Checklist

  • Verify merge order in cmd.go: defaults are written first, then user imports overwrite — confirm this gives user values precedence (not the reverse)
  • Confirm removing ruby-sdk from validate-all-changelogs.sh is correct given the ruby-sdkruby-sdk-v2 migration in chore(ruby): update get-test-matrix to reference ruby-sdk-v2 #14585
  • Consider whether any existing user relies on the old replace-all behavior of module.imports

Testing

  • go build ./... passes
  • go test ./... passes
  • All seed snapshot tests pass (go-sdk, go-model)
  • No unit tests added — internal/cmd has no test infrastructure; relies on seed snapshot tests for regression coverage

Link to Devin session: https://app.devin.ai/sessions/0f8b8b4bcb8c4289828129dc6c59feb5
Requested by: @jsklan


Open with Devin

…f replacing them

Co-Authored-By: judah <jsklan.development@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

ruby-sdk was replaced by ruby-sdk-v2 in PR #14585 but the
validate-all-changelogs.sh script still referenced the old name,
causing the Validate Changelogs CI check to fail for all PRs
touching generators/**/versions.yml.

Co-Authored-By: judah <jsklan.development@gmail.com>
@devin-ai-integration devin-ai-integration bot marked this pull request as ready for review April 3, 2026 23:21
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

@jsklan jsklan merged commit 2870783 into main Apr 6, 2026
150 checks passed
@jsklan jsklan deleted the devin/FER-9453-1775256515 branch April 6, 2026 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants