Avoid manually-coded loops#2087
Conversation
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Looks great to me overall
| } | ||
| options.BigData = append(options.BigData, copyImageBigDataOptionSlice(iOptions.BigData)...) | ||
| options.NamesHistory = append(options.NamesHistory, copyStringSlice(iOptions.NamesHistory)...) | ||
| options.NamesHistory = append(options.NamesHistory, iOptions.NamesHistory...) |
There was a problem hiding this comment.
I only skimmed/spot checked but I noticed in this one we stopped copying...expected?
There was a problem hiding this comment.
Yes — append is “grow slice + copy incoming data to the destination”; so making another copy of the incoming data could only make a difference if the source and destination were overlapping. That’s not the case here (the destination is either unset, or already a freshly-allocated clone).
Actually I’ll remove another copy when updating options.Digests field above.
The BigData one is different - copyImageBigDataOptionSlice is a deep copy, but append only does a shallow one.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, mtrmac The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Conservatively use Index* + Delete to delete the first element where it's not obvious that the code would really want to delete all instances. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use the "slices", "maps" standard library packages, or other readily-available features. Should not change behavior. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
|
/lgtm |
3 participants
Use
slicesandmaps, or other existing features, where appropriate.This is basically a single pass looking for
append.*..., and skimming allforloops.