chore: switch YAML library#3507
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughPinned module Go directives to 1.24.0 and replaced YAML dependency/imports from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 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). (20)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
modules/compose/go.mod (1)
15-15: Direct YAML dependency correctly updated; indirect legacy path is acceptableThis go.mod now pins
go.yaml.in/yaml/v3 v3.0.4directly, withgopkg.in/yaml.v3 v3.0.1only remaining as an indirect dep, which fits the goal of switching your own imports to the maintained module. If you want to be thorough, you could:
- Run
go mod tidyin this module to ensure the indirect block is minimal.- Double‑check that other modules in the repo also converge on the same
go.yaml.in/yaml/v3version to avoid divergence.Also applies to: 191-191
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
modules/k3s/go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
modulegen/go.mod(1 hunks)modulegen/internal/dependabot/reader.go(1 hunks)modulegen/internal/dependabot/writer.go(1 hunks)modulegen/internal/mkdocs/reader.go(1 hunks)modulegen/internal/mkdocs/writer.go(1 hunks)modules/compose/compose_local.go(1 hunks)modules/compose/go.mod(2 hunks)modules/k3s/go.mod(2 hunks)modules/k3s/k3s.go(1 hunks)usage-metrics/go.mod(1 hunks)wait/testdata/http/go.mod(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: mdelapenya
Repo: testcontainers/testcontainers-go PR: 3254
File: .github/dependabot.yml:21-21
Timestamp: 2025-09-18T08:24:27.479Z
Learning: In the testcontainers-go repository, submodules like atlaslocal that are part of a parent module (e.g., mongodb) share the same go.mod file and should not have separate Dependabot entries. They are already monitored through the parent module's Dependabot configuration entry.
📚 Learning: 2025-09-29T15:08:18.694Z
Learnt from: mdelapenya
Repo: testcontainers/testcontainers-go PR: 3320
File: modules/artemis/artemis.go:98-103
Timestamp: 2025-09-29T15:08:18.694Z
Learning: In testcontainers-go, nat.Port is a type alias for string, so untyped string constants can be passed directly to functions expecting nat.Port (like wait.ForListeningPort) without explicit type conversion - the Go compiler handles the implicit conversion automatically.
Applied to files:
modules/k3s/k3s.go
📚 Learning: 2025-09-18T08:24:27.479Z
Learnt from: mdelapenya
Repo: testcontainers/testcontainers-go PR: 3254
File: .github/dependabot.yml:21-21
Timestamp: 2025-09-18T08:24:27.479Z
Learning: In the testcontainers-go repository, submodules like atlaslocal that are part of a parent module (e.g., mongodb) share the same go.mod file and should not have separate Dependabot entries. They are already monitored through the parent module's Dependabot configuration entry.
Applied to files:
modulegen/go.modwait/testdata/http/go.modusage-metrics/go.modmodules/compose/go.mod
⏰ 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). (20)
- GitHub Check: lint (modules/dockermodelrunner) / lint: modules/dockermodelrunner
- GitHub Check: lint (modules/dind) / lint: modules/dind
- GitHub Check: lint (modules/arangodb) / lint: modules/arangodb
- GitHub Check: lint (modules/cockroachdb) / lint: modules/cockroachdb
- GitHub Check: lint (modules/mssql) / lint: modules/mssql
- GitHub Check: lint (modules/toxiproxy) / lint: modules/toxiproxy
- GitHub Check: lint (modules/couchbase) / lint: modules/couchbase
- GitHub Check: lint (modules/socat) / lint: modules/socat
- GitHub Check: lint (modules/artemis) / lint: modules/artemis
- GitHub Check: lint (modules/etcd) / lint: modules/etcd
- GitHub Check: lint (modules/memcached) / lint: modules/memcached
- GitHub Check: lint (modules/scylladb) / lint: modules/scylladb
- GitHub Check: lint (modules/pulsar) / lint: modules/pulsar
- GitHub Check: lint (modules/azurite) / lint: modules/azurite
- GitHub Check: lint (modules/yugabytedb) / lint: modules/yugabytedb
- GitHub Check: lint (modules/ollama) / lint: modules/ollama
- GitHub Check: lint (modules/consul) / lint: modules/consul
- GitHub Check: lint (modules/k3s) / lint: modules/k3s
- GitHub Check: lint (modules/gcloud) / lint: modules/gcloud
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
wait/testdata/http/go.mod (1)
3-5: Consistent Go version directive for HTTP wait testdataUpdating to
go 1.24.0withtoolchain go1.24.7follows the expected pattern for go.mod updates in the project.usage-metrics/go.mod (1)
3-5: Go version directive update is acceptableThe change to
go 1.24.0withtoolchain go1.24.7appears reasonable for pinning dependencies. Ensure this version combination is consistent with other go.mod files in the workspace if this is a multi-module project.modules/k3s/k3s.go (1)
13-18: K3s YAML provider swap is localized and preserves behaviorImporting
go.yaml.in/yaml/v3while keeping themarshal/unmarshalhelpers usingyaml.Marshalandyaml.Unmarshalmaintains existing behavior; the only effective change is which YAML module provides the implementation.Verify the dependency wiring in
modules/k3s/go.modcontains the new YAML path and confirm no legacygopkg.in/yaml.v3imports remain elsewhere in the codebase.Also applies to: 165-180
modules/compose/compose_local.go (1)
17-17: YAML import path swap in compose looks correctThe switch to
go.yaml.in/yaml/v3while keepingyaml.Unmarshalusage invalidate()unchanged preserves behavior and aligns with the PR's goal of using the maintained YAML module.To ensure everything is wired correctly and no legacy imports remain, verify that:
modules/compose/go.modincludes the correctgo.yaml.in/yaml/v3dependency- No lingering
gopkg.in/yaml.v3imports exist elsewhere in the codebasemodulegen/internal/dependabot/reader.go (1)
7-21: Unable to verify review comment due to repository access constraintsThe review comment cannot be verified without direct access to the codebase. Before approval, confirm: (1) the import path is
go.yaml.in/yaml/v3, (2)modulegen/go.modincludes this dependency, and (3) the module builds successfully withgo test ./... -mod=readonlyfrom the modulegen directory.modulegen/internal/mkdocs/reader.go (1)
7-21: MkDocs config reader now uses the new YAML moduleThe import path has been switched to
go.yaml.in/yaml/v3while theyaml.Unmarshalcall and error handling remain unchanged, preserving the function's semantics.To complete this change, verify that
modulegen/go.modincludes the new YAML dependency and that all oldgopkg.in/yaml.v3imports have been removed from the modulegen package.modules/k3s/go.mod (1)
12-16: Direct YAML dependency updated; legacy gopkg.in remains only as indirectAdding
go.yaml.in/yaml/v3 v3.0.4as a direct requirement correctly matches the updated imports ink3s.go. Keepinggopkg.in/yaml.v3 v3.0.1 // indirectis fine if it's required transitively, though it means the unmaintained path still exists in the graph via dependencies rather than your code.As a low‑priority cleanup, you might track which module still pulls in
gopkg.in/yaml.v3and eventually bump/replace it once upstream migrates.modulegen/go.mod (1)
3-12: modulegen YAML dependency path may be out of sync with importsIf
modulegen/internal/{mkdocs,dependabot}are importinggo.yaml.in/yaml/v3whilemodulegen/go.modrequiresgopkg.in/yaml.v3 v3.0.1, this mismatch will causego build/go testwith-mod=readonlyto fail. The two module paths use different import semantics and cannot coexist in the dependency graph.Consider one of these approaches:
Update to canonical path: Replace
gopkg.in/yaml.v3 v3.0.1withgo.yaml.in/yaml/v3 v3.0.4(or the version used inmodules/k3s/go.mod), then rungo mod tidy.Use a replace directive: If other modules depend on
gopkg.in/yaml.v3, add a replace directive tomodulegen/go.mod:replace gopkg.in/yaml.v3 => go.yaml.in/yaml/v3 v3.0.4Then run
go mod tidy.Verify which import path
modulegen/internalactually uses and standardize accordingly across the codebase to avoid similar mismatches in the future.modulegen/internal/mkdocs/writer.go (1)
3-10: YAML import swap is consistent and non-invasiveThe switch to
go.yaml.in/yaml/v3while keepingyaml.Marshal(config)unchanged aligns with the PR goal and should be behavior‑preserving here, assuming the module is correctly added in go.mod.modulegen/internal/dependabot/writer.go (1)
3-9: Dependabot writer correctly migrated to new YAML moduleImporting
go.yaml.in/yaml/v3while keepingyaml.Marshal(config)as‑is matches the intended dependency switch and keeps this writer’s behavior intact.
The former is unmaintained.
This is to pick up spf13/cobra#2336 to have one less dependency on spf13/cobra#2336.
| github.com/pmezard/go-difflib v1.0.0 // indirect | ||
| github.com/spf13/pflag v1.0.5 // indirect | ||
| github.com/spf13/pflag v1.0.9 // indirect | ||
| gopkg.in/yaml.v3 v3.0.1 // indirect |
There was a problem hiding this comment.
The old dep is still pulled in via testify. It's being tracked in stretchr/testify#1772.
mdelapenya
left a comment
mdelapenya
left a comment
There was a problem hiding this comment.
LGTM! Thank you for this. I was aware of the Yml library being unmaintained, and this PR is good news.
2 participants
What does this PR do?
The original
gopkg.in/yaml.v3was marked unmaintained (April 2025). See https://github.com/go-yaml/yaml.go.yaml.in/yaml/v3is now maintained by the official YAML organization and is a drop-in replacement with an identical API.Please see individual commits.
Why is it important?
Nobody will fix issues in the unmaintained dependency.
Related issues
This is similar to spf13/cobra#2336.