Conversation
Modernize Go code using go fix: interface{} to any, reflect.Ptr to
reflect.Pointer, reflect.TypeOf to reflect.TypeFor, integer range syntax,
maps.Copy, slices.Contains, strings.SplitSeq, strings.CutPrefix, and
fmt.Appendf.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Go codebase using go fix-style updates and newer standard library APIs (e.g., maps, slices, strings sequence helpers), while also aligning types toward any and newer reflection helpers.
Changes:
- Replace
interface{}withanyacross production and test code. - Adopt newer stdlib helpers (
maps.Copy,slices.Contains/ContainsFunc,strings.CutPrefix,strings.SplitSeq,fmt.Appendf) and integer-range loops (for i := range n). - Update reflection-related code to use newer reflect constants/helpers (e.g.,
reflect.Pointer,reflect.TypeFor).
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| workflow_test.go | Uses integer-range loops in concurrency/parallelism tests. |
| step_test.go | Switches to strings.SplitSeq iteration for line checking. |
| ssh/client_test.go | Uses maps.Copy for test input map cloning. |
| ssh/client.go | Uses strings.CutPrefix and maps.Copy for request preparation/copying. |
| shell/client.go | Uses slices.Contains and strings.CutPrefix for validation and env extraction. |
| repeat_test.go | Updates tests to any (including pointer cases). |
| repeat.go | Updates YAML marshal/unmarshal signatures/return types to any. |
| probe.go | Uses strings.SplitSeq for multi-path parsing. |
| printer_test.go | Updates varargs/types to any and SplitSeq-based iteration. |
| printer.go | Updates public printer APIs to ...any and uses SplitSeq for echo formatting. |
| outputs.go | Uses maps.Copy for copying output maps. |
| mapping.go | Uses maps.Copy, reflect.TypeFor, and reflect.Pointer in mapping utilities. |
| mail/mock_server.go | Uses strings.SplitSeq for CRLF command splitting. |
| mail/bulk.go | Replaces string concatenation loop with strings.Builder. |
| imap/client.go | Uses maps.Copy, strings.SplitSeq, and strings.CutPrefix in parsing/fetch logic. |
| http/client_test.go | Updates test header maps to map[string]any. |
| http/client.go | Uses maps.Copy and updates header map type assertions to map[string]any. |
| grpc/client.go | Uses maps.Copy for request map cloning. |
| funcs.go | Updates helper signature to any. |
| expr_test.go | Uses integer-range loops in edge-case tests. |
| expr.go | Uses slices.Contains, fmt.Appendf for byte formatting in templating. |
| error.go | Updates error context type and context setter to any. |
| embedded/client.go | Uses maps.Copy to avoid mutating caller request maps. |
| db/client_test.go | Updates params/rows types in tests to any. |
| db/client.go | Updates DB request/response types and callback signatures to any. |
| dag_ascii.go | Uses slices.Contains for membership checks in DAG rendering. |
| dag/cycles.go | Uses slices.ContainsFunc for DFS-driven cycle detection. |
| cmd/probe/main.go | Uses slices.Contains and index-range loop modernization in CLI parsing. |
| browser/client_test.go | Updates debug callback varargs to ...any. |
| browser/client.go | Updates debug callback varargs to ...any. |
| ascii-dag/layout.go | Uses integer-range loop in level calculation. |
| ascii-dag/dag.go | Uses slices.Contains to prevent duplicate edges. |
| actions_test.go | Uses maps.Copy for copying action params into results. |
| actions/ssh/main.go | Updates log map type to map[string]any. |
| actions/hello/main.go | Uses maps.Copy for response map construction. |
| actions/db/main.go | Updates callback signature to []any. |
| actions/browser/main.go | Updates callback varargs to ...any. |
| actions.go | Uses reflect.Pointer when detecting pointer values for protobuf conversion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mapping.go
Outdated
| } else if field.Type() == reflect.TypeFor[map[string]string]() { | ||
| v, ok := params[mapTag].(map[string]any) | ||
| if !ok && validateTag == labelRequired { | ||
| return fmt.Errorf("expected map[string]interface{} for field '%s'", mapTag) |
There was a problem hiding this comment.
The error message still says map[string]interface{} even though this function now uses map[string]any elsewhere. Since any is the preferred spelling in this file, consider updating the message for consistency and easier debugging.
Suggested change
| return fmt.Errorf("expected map[string]interface{} for field '%s'", mapTag) | |
| return fmt.Errorf("expected map[string]any for field '%s'", mapTag) |
Comment on lines
+165
to
+170
| var result strings.Builder | ||
| for i := 0; i < n; i += length { | ||
| if i+length < len(s) { | ||
| result += s[i:i+length] + "\n" | ||
| result.WriteString(s[i:i+length] + "\n") | ||
| } else { | ||
| result += s[i:] | ||
| result.WriteString(s[i:]) |
There was a problem hiding this comment.
strings.Builder is a good change, but result.WriteString(s[i:i+length] + "\n") still allocates a temporary concatenated string each iteration. To keep this allocation-free, write the substring and newline separately (and optionally Grow the builder if you can estimate the final size).
- Update error message from map[string]interface{} to map[string]any in
mapping.go for consistency
- Avoid temporary string allocation in strings.Builder by splitting
WriteString and WriteByte calls in mail/bulk.go
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Code Metrics Report
Details | | main (3f2408c) | #169 (69facee) | +/- |
|---------------------|----------------|----------------|--------|
- | Coverage | 56.0% | 55.9% | -0.1% |
| Files | 65 | 65 | 0 |
| Lines | 6579 | 6548 | -31 |
- | Covered | 3685 | 3662 | -23 |
+ | Code to Test Ratio | 1:1.0 | 1:1.0 | +0.0 |
| Code | 12888 | 12848 | -40 |
- | Test | 13269 | 13267 | -2 |
+ | Test Execution Time | 1m25s | 11s | -1m14s |Code coverage of files in pull request scope (56.3% → 56.1%)
Reported by octocov |
Owner
Author
|
/run-e2e |
|
❌ E2E test failed. View logs |
|
✅ E2E test passed successfully! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
go fixmodernization across the entire codebaseinterface{}withany,reflect.Ptrwithreflect.Pointer,reflect.TypeOfwithreflect.TypeFormaps.Copy,slices.Contains,strings.SplitSeq,strings.CutPrefix,fmt.Appendffor i := range n)Test plan
go build ./...passesgo vet ./...passesgo test ./...all tests pass🤖 Generated with Claude Code