Modernize dm (excluding dm/pkg)#12719
Conversation
ti-chi-bot
Bot
added
the
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
area/dm
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modernizes the codebase by adopting Go 1.21 and 1.22 features, such as replacing interface{} with any, using for range over integers, utilizing maps.Copy and slices.Contains, and leveraging fmt.Appendf. However, a critical issue was identified across multiple files where standard sync.WaitGroup usages were incorrectly refactored to use a non-existent Go method (e.g., wg.Go(func() { ... })). This will result in compilation failures, and these changes must be reverted to the standard wg.Add(1) and go func() pattern.
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.
| clone.Session = make(map[string]string, len(db.Session)) | ||
| for k, v := range db.Session { | ||
| clone.Session[k] = v | ||
| } | ||
| maps.Copy(clone.Session, db.Session) |
There was a problem hiding this comment.
| clone.Session = make(map[string]string, len(db.Session)) | |
| for k, v := range db.Session { | |
| clone.Session[k] = v | |
| } | |
| maps.Copy(clone.Session, db.Session) | |
| clone.Session = maps.Clone(db.Session) |
| for item, desc := range AllCheckingItems { | ||
| checkingItems[item] = desc | ||
| } | ||
| maps.Copy(checkingItems, AllCheckingItems) |
| for item, desc := range AllCheckingItems { | ||
| checkingItems[item] = desc | ||
| } | ||
| maps.Copy(checkingItems, AllCheckingItems) |
| inSlice := func(s []string, e string) bool { | ||
| for _, v := range s { | ||
| if v == e { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| return slices.Contains(s, e) | ||
| } | ||
| for _, workerResp := range resps { | ||
| workerRespMap[workerResp.SourceStatus.Source] = append(workerRespMap[workerResp.SourceStatus.Source], workerResp) | ||
| // append some offline worker responses | ||
| if !inSlice(sources, workerResp.SourceStatus.Source) { |
There was a problem hiding this comment.
remove the inSlice closure and make line 848 call slices.Contains directly
| value: make(map[string]any), | ||
| } | ||
| maps.Copy(result.value, value) |
There was a problem hiding this comment.
| value: make(map[string]any), | |
| } | |
| maps.Copy(result.value, value) | |
| value: maps.Clone(value), | |
| } |
| uk.value[k] = v | ||
| } | ||
| uk.value = make(map[string]any) | ||
| maps.Copy(uk.value, value) |
| result.value[k] = v | ||
| value: map[string]any{}, | ||
| } | ||
| maps.Copy(result.value, uk.value) |
| for k, v := range sg.sources { | ||
| ret[k] = v | ||
| } | ||
| maps.Copy(ret, sg.sources) |
| for key, value := range k.groups { | ||
| groups[key] = value | ||
| } | ||
| maps.Copy(groups, k.groups) |
| for name, st := range h.subTasks { | ||
| result[name] = st | ||
| } | ||
| maps.Copy(result, h.subTasks) |
2 participants
What problem does this PR solve?
Issue Number: ref #12691
What is changed and how it works?
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note