Refactor copy of slices and maps#2128
Refactor copy of slices and maps#2128openshift-merge-bot[bot] merged 2 commits intocontainers:mainfrom
Conversation
a9ef2aa to
13a96fe
Compare
store.go
Outdated
| options.BigData = copyContainerBigDataOptionSlice(cOptions.BigData) | ||
| options.BigData = slices.Clone(cOptions.BigData) |
There was a problem hiding this comment.
slices.Clone makes a shallow clone, i.e. Data slice is not really copied with this change.
There was a problem hiding this comment.
The previous copyContainerBigDataOptionSlice function used the slices.Clone function to copy data. So I would expect similar behavior.
There was a problem hiding this comment.
It used slices.Clone() to copy the Data field in each element in the slice. The godoc for slices.Clone() notes that it only copies using assignment, so a new slice would not be created for Data fields.
There was a problem hiding this comment.
Okay, I've reverted these changes.
|
/approved |
13a96fe to
b1a1078
Compare
Removes duplicate copy*Slice functions using a generic copy function or replaces them with the slices.Clone function. Also simplifies the stringSliceWithoutValue function. These changes should not change the behavior. Signed-off-by: Jan Rodák <hony.com@seznam.cz>
Removes the duplicate copy*Map function using the general function newMapFrom. Reduces the allocation of empty maps using the copyMapPrefferingNil function. This change may affect the behavior so that instead of an empty allocated map, a nil will be returned. Signed-off-by: Jan Rodák <hony.com@seznam.cz>
b1a1078 to
43d0009
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Honny1, rhatdan 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 |
|
LGTM |
|
/lgtm |
This PR refactors:
slices.Clonefunction.stringSliceWithoutValuefunction.newMapFrom.copyMapPrefferingNilfunction.nilwill be returned.Follows up on: #2122