Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove util/slice and use standard slices library #13775

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Joibel
Copy link
Member

@Joibel Joibel commented Oct 17, 2024

Motivation

/util/slices can be replaced by the official slices library added in go 1.21. This library is also the official version of golang.org/x/exp/slices.

Replace all uses of these libraries with slices

Modifications

Replace slice.ContainsString() with slices.Contains(). This is very simple.

Replace slice.RemoveString() with slices.DeleteFunc(), which involves a small inline boolean comparison function. RemoveString returned a copy of the slice, DeleteFunc modifies the slice in place. In the one case where we're using RemoveString to clone the slice (getPodCleanupPatch), slices.Clone() it before modifying.

Also fix a test name with a spelling mistake.

Verification

Test suite to catch the problems

@Joibel Joibel changed the title refactor: remove util/slice and use standard slices library refactor: remove util/slice and use standard slices library Oct 17, 2024
@Joibel Joibel marked this pull request as ready for review October 17, 2024 15:58
@Joibel
Copy link
Member Author

Joibel commented Oct 17, 2024

Draft as it collides with #13776/#12862

@Joibel Joibel marked this pull request as draft October 17, 2024 16:56
Signed-off-by: Alan Clucas <alan@clucas.org>
@Joibel
Copy link
Member Author

Joibel commented Oct 22, 2024

Draft as it collides with #13776/#12862

This is now merged and this rebased on top.

@Joibel Joibel marked this pull request as ready for review October 22, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant