Skip to content

Run go fix with go1.26#12715

Closed
dveeden wants to merge 9 commits into
pingcap:masterfrom
dveeden:modern_go_126
Closed

Run go fix with go1.26#12715
dveeden wants to merge 9 commits into
pingcap:masterfrom
dveeden:modern_go_126

Conversation

@dveeden

@dveeden dveeden commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: ref #12691

What is changed and how it works?

Same as #12692 but with Go 1.26 and without removing the GODEBUG options that are removed in Go 1.27.

This can be applied to make the change in #12692 smaller

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note

None

@ti-chi-bot ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. affect-ticdc-config-docs Pull requests that affect TiCDC configuration docs. area/dm Issues or PRs related to DM. area/engine Issues or PRs related to Dataflow Engine. labels Jun 15, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign lance6716, yudongusa for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 15, 2026
@dveeden

dveeden commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/cc @OliverS929 @kennytm

@ti-chi-bot ti-chi-bot Bot requested review from OliverS929 and kennytm June 15, 2026 08:09

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request performs a large-scale refactoring across the codebase, replacing interface{} with any, adopting Go 1.21+ features like slices.Sort, slices.Contains, and maps.Copy, and simplifying loops using for range. However, several critical compilation errors were introduced where a non-existent Go method is called on sync.WaitGroup instead of errgroup.Group. Additionally, there are opportunities to optimize string building using fmt.Fprintf on strings.Builder and to simplify boolean assignments using slices.Contains directly.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread cdc/kv/sharedconn/conn_and_client_test.go
Comment thread cdc/processor/memquota/mem_quota_test.go
Comment thread cmd/kafka-consumer/main.go
Comment thread dm/pkg/conn/basedb.go Outdated
Comment thread cdc/api/middleware/middleware.go Outdated
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.51908% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.8235%. Comparing base (e4a15d2) to head (fd17416).
✅ All tests successful. No failed tests found.

Additional details and impacted files
Components Coverage Δ
cdc 57.1638% <76.6990%> (-0.0621%) ⬇️
dm ∅ <ø> (∅)
engine 50.5654% <57.1428%> (-0.0893%) ⬇️
Flag Coverage Δ
cdc 57.1638% <76.6990%> (?)
unit 55.8235% <72.5190%> (+2.1993%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@               Coverage Diff                @@
##             master     #12715        +/-   ##
================================================
+ Coverage   53.6242%   55.8235%   +2.1993%     
================================================
  Files          1038        737       -301     
  Lines        146348      87069     -59279     
================================================
- Hits          78478      48605     -29873     
+ Misses        61961      35005     -26956     
+ Partials       5909       3459      -2450     
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dveeden

dveeden commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/cc @wuhuizuo

@ti-chi-bot ti-chi-bot Bot requested a review from wuhuizuo June 15, 2026 08:47

@kennytm kennytm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

~380 files remain unreviewed, i'll check the rest in the next batch tomorrow

preferably tell your Claude to fix those maps.Copymaps.Clone and sb.WriteString(fmt.Sprintf(...))fmt.Fprintf(&sb, ...) in all other files.

Comment thread cdc/model/owner.go Outdated
Comment thread cdc/api/middleware/middleware.go Outdated
Comment thread cdc/scheduler/internal/v3/replication/replication_set_test.go Outdated
Comment thread cmd/oauth2-server/main.go Outdated
Comment thread cmd/storage-consumer/main.go Outdated
Comment thread dm/pkg/conn/basedb.go
setFK = true
}
dsn += fmt.Sprintf("&%s='%s'", key, url.QueryEscape(val))
dsn.WriteString(fmt.Sprintf("&%s='%s'", key, url.QueryEscape(val)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
dsn.WriteString(fmt.Sprintf("&%s='%s'", key, url.QueryEscape(val)))
fmt.Fprintf(&dsn, "&%s='%s'", key, url.QueryEscape(val))

Comment thread dm/pkg/shardddl/optimism/keeper.go Outdated
Comment on lines +168 to +169
locks := make(map[string]*Lock, len(lk.locks))
for k, v := range lk.locks {
locks[k] = v
}
maps.Copy(locks, lk.locks)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
locks := make(map[string]*Lock, len(lk.locks))
for k, v := range lk.locks {
locks[k] = v
}
maps.Copy(locks, lk.locks)
locks := maps.Clone(lk.locks)

}
}
return false
return slices.Contains(s, e)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

better find out who calls contains and replace them with slice.Contains

Comment thread dm/pkg/shardddl/pessimism/keeper.go Outdated
Comment on lines +88 to +89
locks := make(map[string]*Lock, len(lk.locks))
for k, v := range lk.locks {
locks[k] = v
}
maps.Copy(locks, lk.locks)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
locks := make(map[string]*Lock, len(lk.locks))
for k, v := range lk.locks {
locks[k] = v
}
maps.Copy(locks, lk.locks)
locks := maps.Clone(lk.locks)

Comment thread dm/pkg/shardddl/pessimism/lock.go Outdated
Comment on lines +140 to +141
ret := make(map[string]bool, len(l.ready))
for k, v := range l.ready {
ret[k] = v
}
maps.Copy(ret, l.ready)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
ret := make(map[string]bool, len(l.ready))
for k, v := range l.ready {
ret[k] = v
}
maps.Copy(ret, l.ready)
ret := maps.Clone(l.ready)

dveeden and others added 7 commits June 16, 2026 08:12
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: kennytm <kennytm@gmail.com>
Co-authored-by: kennytm <kennytm@gmail.com>
Replace the make + maps.Copy pattern with maps.Clone where a fresh map
is created and fully populated from a source map. Applies the PR review
suggestions and the same simplification found elsewhere in the codebase.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ti-chi-bot

ti-chi-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

@dveeden: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-error-log-review bc914b5 link true /test pull-error-log-review
pull-check bc914b5 link true /test pull-check
pull-cdc-integration-storage-test bc914b5 link true /test pull-cdc-integration-storage-test
pull-cdc-integration-pulsar-test bc914b5 link true /test pull-cdc-integration-pulsar-test
pull-cdc-integration-kafka-test bc914b5 link true /test pull-cdc-integration-kafka-test
pull-cdc-integration-mysql-test bc914b5 link true /test pull-cdc-integration-mysql-test
pull-dm-integration-test bc914b5 link true /test pull-dm-integration-test

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@dveeden

dveeden commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

I'm closing this PR and I'm going to split it up into multiple PRs:

  • Makes it easier to review
  • Required for the pull-error-log-review check
  • Makes approval easier

@dveeden dveeden closed this Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

affect-ticdc-config-docs Pull requests that affect TiCDC configuration docs. area/dm Issues or PRs related to DM. area/engine Issues or PRs related to Dataflow Engine. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants