chore: bump Go 1.19→1.25, replace k8s dep, remove golangci-lint from …#22
Conversation
…go.mod
- Bump go directive to 1.25
- Replace k8s.io/apimachinery sets.String with internal stringset.go (sheds massive k8s dep tree)
- Remove golangci-lint from tools/tools.go (keep gomarkdoc); sheds ~170 indirect deps
- Replace 22 interface{} → any across 5 files
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR updates CI and the Go toolchain to 1.25, replaces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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 |
There was a problem hiding this comment.
Pull request overview
This PR updates the module to a newer Go version, reduces dependency footprint by removing Kubernetes apimachinery (replacing sets.String with an internal implementation), and modernizes a few type usages (interface{} → any) across the public API and internals.
Changes:
- Bump the Go version and refresh module dependencies (including
tracingandgo-graphviz). - Replace
k8s.io/apimachinery/pkg/util/sets.Stringusage with an internalstringSetto shed the k8s dependency tree. - Remove
golangci-lintfrom tracked tool dependencies and migrateinterface{}toany.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
go.mod |
Updates Go directive and dependency versions; removes k8s dependency and golangci-lint. |
stringset.go |
Adds internal stringSet implementation to replace k8s sets.String. |
dependency.go |
Switches dependency-resolution set logic from k8s sets.String to stringSet. |
plan.go |
Removes k8s set usage, updates result/init maps to any, and updates graphviz construction/rendering API usage. |
databuilder.go |
Updates builder storage and set tracking to any/stringSet, removing k8s import. |
types.go |
Updates public interfaces to use any instead of interface{}. |
context.go |
Updates GetFromResult signature to any. |
tools/tools.go |
Removes golangci-lint from tools tracking imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| module github.com/go-coldbrew/data-builder | ||
|
|
||
| go 1.19 | ||
| go 1.25.0 |
There was a problem hiding this comment.
go directive should use the Go language version in major.minor form; go 1.25.0 is rejected by the go command (expects e.g. go 1.25). Update this line to a valid format so module tooling can parse the file.
| go 1.25.0 | |
| go 1.25 |
| readyset := newStringSet(initData...) | ||
| order := make([][]*builder, 0) | ||
| for len(structMap) > 0 { | ||
| blocked := sets.NewString() | ||
| blocked := newStringSet() | ||
| for k, v := range structMap { | ||
| if v.Len() == 0 { |
There was a problem hiding this comment.
Switching from sets.String to stringSet removes the String() implementation that k8s sets provide. Later in this function, blocked is formatted with %s, which will now produce a %!s(...) formatting error instead of a readable list of missing fields. Add a String() method on stringSet (to preserve existing error text behavior) or update the error formatting to use blocked.List() / %v.
Go 1.26 toolchain normalizes go directive to 1.25.0; upgrade actions/checkout to v4 and actions/setup-go to v5 so CI installs a Go version that understands this format.
The stringSet type replaced k8s sets.String but was missing a String() method. This caused %s formatting in error messages to produce unreadable output instead of the expected [a b c] format.
- Rename local var 'max' to 'maxParallel' to avoid shadowing Go 1.21+ builtin. - Replace TODO comments with descriptive comments. - Update go-git/go-git v5.3→v5.17 (fixes CVE-2023-49569, CVE-2025-21613, CVE-2023-49568, CVE-2025-21614). - Update logrus v1.8→v1.9.4 (fixes CVE-2025-65637).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
stringset.go (1)
57-68: Consider usingstrings.Builderfor better performance.String concatenation in a loop creates a new string on each iteration, resulting in O(n²) complexity. Using
strings.Builderwould be O(n).♻️ Suggested improvement
+import ( + "sort" + "strings" +) + // String returns a human-readable representation of the set, e.g. "[a b c]". func (s stringSet) String() string { items := s.List() - result := "[" - for i, item := range items { - if i > 0 { - result += " " - } - result += item - } - result += "]" - return result + var b strings.Builder + b.WriteByte('[') + for i, item := range items { + if i > 0 { + b.WriteByte(' ') + } + b.WriteString(item) + } + b.WriteByte(']') + return b.String() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stringset.go` around lines 57 - 68, The stringSet.String() implementation builds the output via repeated string concatenation which is quadratic; replace the loop with a strings.Builder: import "strings", create a builder, WriteString the "[" prefix, loop over items := s.List() and write a space separator and each item via builder.WriteString, then append "]" and return builder.String(); keep function name stringSet.String and use s.List() to obtain items.stringset_test.go (1)
10-32: Good test coverage forString()method.The tests correctly verify:
- Empty set formatting
- Single element formatting
- Multiple elements with deterministic sorting
fmt.Stringerinterface integrationConsider adding tests for
Has,Insert,IsSuperset,Difference, andLenmethods to ensure comprehensive coverage of the newstringSettype.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stringset_test.go` around lines 10 - 32, Add unit tests for stringSet behavior beyond String(): create tests using newStringSet(...) and assert Has, Insert, Len, IsSuperset, and Difference semantics; specifically add table-driven cases that verify Insert adds items and updates Len and Has, Has returns false for absent items, IsSuperset correctly identifies superset/subset relations between stringSet instances, and Difference returns the expected set of items (and does not mutate operands). Use the existing test style (testing.T, t.Run, assert.Equal/assert.True/assert.False) and reference the constructors and methods newStringSet, Insert, Has, Len, IsSuperset, and Difference when locating code to test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@go.mod`:
- Line 3: Update the Go module directive to the canonical format by changing the
go directive value from "1.25.0" to "1.25" in the go.mod file; locate the go
directive (the line that currently reads go 1.25.0) and replace the patch
version with the major.minor version so it reads go 1.25 to satisfy the Go
modules specification.
- Line 27: Update the go-git dependency from github.com/go-git/go-git/v5 v5.3.0
to v5.17.0 to pull in security fixes; modify the version string for the module
reference (github.com/go-git/go-git/v5) in go.mod and then run the module update
commands (e.g., go get github.com/go-git/go-git/v5@v5.17.0 and go mod tidy) so
the lockfile and vendor state are consistent.
- Line 50: Update the logrus dependency in go.mod from
github.com/sirupsen/logrus v1.8.1 to a fixed version (at least v1.8.3,
preferably v1.9.4) to mitigate CVE-2025-65637; after editing the dependency
entry for github.com/sirupsen/logrus, run the Go module update commands (eg. go
get github.com/sirupsen/logrus@v1.9.4 and go mod tidy) so go.sum is refreshed
and the new version is used throughout the build.
---
Nitpick comments:
In `@stringset_test.go`:
- Around line 10-32: Add unit tests for stringSet behavior beyond String():
create tests using newStringSet(...) and assert Has, Insert, Len, IsSuperset,
and Difference semantics; specifically add table-driven cases that verify Insert
adds items and updates Len and Has, Has returns false for absent items,
IsSuperset correctly identifies superset/subset relations between stringSet
instances, and Difference returns the expected set of items (and does not mutate
operands). Use the existing test style (testing.T, t.Run,
assert.Equal/assert.True/assert.False) and reference the constructors and
methods newStringSet, Insert, Has, Len, IsSuperset, and Difference when locating
code to test.
In `@stringset.go`:
- Around line 57-68: The stringSet.String() implementation builds the output via
repeated string concatenation which is quadratic; replace the loop with a
strings.Builder: import "strings", create a builder, WriteString the "[" prefix,
loop over items := s.List() and write a space separator and each item via
builder.WriteString, then append "]" and return builder.String(); keep function
name stringSet.String and use s.List() to obtain items.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 576bf495-d17f-4ca6-bdd7-1bdb81a67ea2
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
.github/workflows/go.ymlcontext.godatabuilder.godependency.gogo.modplan.gostringset.gostringset_test.gotools/tools.gotypes.go
💤 Files with no reviewable changes (1)
- tools/tools.go
| module github.com/go-coldbrew/data-builder | ||
|
|
||
| go 1.19 | ||
| go 1.25.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Has Go 1.25 been released and what is the correct go.mod directive format for Go 1.25?
💡 Result:
Yes, Go 1.25 was released on August 12, 2025. The correct go.mod directive format for Go 1.25 is the standard 'go 1.25' line, unchanged from previous versions. There are no special syntax changes for the go directive in Go 1.25; it follows the established format as documented in the official Go modules reference: go 1.25. New features like the go.mod ignore directive exist, but the Go version directive remains go . When using go mod init with Go 1.26+, it may default to go 1.25.0 for compatibility, but can be set explicitly with go mod edit -go=1.25 or go get go@1.25.x.
Citations:
- 1: https://go.dev/doc/go1.25
- 2: https://go.dev/blog/go1.25
- 3: https://go.dev/doc/devel/release
- 4: https://go.dev/doc/modules/gomod-ref
- 5: https://go.dev/doc/go1.26
Change go 1.25.0 to go 1.25 to match the official Go modules specification.
The go directive format must be go <version> without a patch number. The pipeline failure occurs because 1.25.0 does not match the expected format pattern. Update line 3 to go 1.25.
🧰 Tools
🪛 GitHub Actions: PR `#22`
[error] 3-3: go.mod parse failed: invalid go version '1.25.0' at /home/runner/work/data-builder/data-builder/go.mod:3. Must match format 1.23.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@go.mod` at line 3, Update the Go module directive to the canonical format by
changing the go directive value from "1.25.0" to "1.25" in the go.mod file;
locate the go directive (the line that currently reads go 1.25.0) and replace
the patch version with the major.minor version so it reads go 1.25 to satisfy
the Go modules specification.
Summary by CodeRabbit
Chores
Tests