Skip to content

Simplify some code with slices and for-range.#1237

Open
atombrella wants to merge 1 commit intocontainernetworking:mainfrom
atombrella:feature/modernize_update
Open

Simplify some code with slices and for-range.#1237
atombrella wants to merge 1 commit intocontainernetworking:mainfrom
atombrella:feature/modernize_update

Conversation

@atombrella
Copy link
Copy Markdown
Contributor

I applied a few of the modernize rules to simplify the code slightly:

  • slices.Contains removes some chunks of iterations that do exactly what can be leveraged to slices.Contains
  • for range N which is syntactic sugar.
  • A single // +build which definitely needs to go.
  • slices.Sort instead of sort.Slice.

The golangci-lint version is a bit old, and I didn't want to tamper around with it in this PR.

@atombrella atombrella force-pushed the feature/modernize_update branch 2 times, most recently from afa7a75 to 2d7d599 Compare March 8, 2026 21:35
@atombrella
Copy link
Copy Markdown
Contributor Author

@s1061123 Care to have a look? I could enable the modernize linter with some rules in golangci-lint.yaml

@s1061123
Copy link
Copy Markdown
Contributor

I don't have so much time these days, hence I recommend to find another guy to review...

@s1061123
Copy link
Copy Markdown
Contributor

In addition, this PR and your another one seems to be 'golang syntax improvement to modern', hence the priority looks lower than other serious one (such as bug fix and security one and so on).
Currently unfortunately we need a lot of such tasks, so please wait for a while.

@squeed
Copy link
Copy Markdown
Member

squeed commented Apr 13, 2026

@atombrella this looks good to me. Can you rebase without a merge commit and we can merge this?

Signed-off-by: Mads Jensen <atombrella@users.noreply.github.com>
@atombrella atombrella force-pushed the feature/modernize_update branch from 708642f to 1712038 Compare April 13, 2026 18:02
@atombrella
Copy link
Copy Markdown
Contributor Author

atombrella commented Apr 13, 2026

@atombrella this looks good to me. Can you rebase without a merge commit and we can merge this?

@squeed Rebased 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants