Skip to content

Refactor ErrorLists into nil slices #2598

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 17, 2025
Merged

Refactor ErrorLists into nil slices #2598

merged 1 commit into from
Apr 17, 2025
+99 −99

Conversation

mflendrich
Copy link
Collaborator

Tiny drive-by refactor to use nil slices where they're perfectly legal, and marginally better memory allocation-wise.
I've grepped k8s.io/apimachinery and they're somewhat 50-50 divided on "nil slice" and allErrs := field.ErrorList{}.

I was somewhat on the edge of "if it ain't broken, don't fix it" but uncontroversial little code smells might be a good way to get the code health in a better shape.

Feel free to close if you see a good reason to leave it as it was.

/cc rzetelskik
/kind cleanup
/priority backlog

@mflendrich mflendrich requested a review from rzetelskik April 16, 2025 13:49
@scylla-operator-bot scylla-operator-bot bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. priority/backlog Higher priority than priority/awaiting-more-evidence. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 16, 2025
@mflendrich mflendrich mentioned this pull request Apr 16, 2025
Merged
@mflendrich
Refactor ErrorLists into nil slices
Loading
Loading status checks…
3e6be47
rzetelskik
rzetelskik approved these changes Apr 16, 2025
View reviewed changes
Copy link
Member

@rzetelskik rzetelskik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@scylla-operator-bot scylla-operator-bot bot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2025
@scylla-operator-bot scylla-operator-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mflendrich, rzetelskik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [mflendrich,rzetelskik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mflendrich
Copy link
Collaborator Author

mflendrich commented Apr 16, 2025 via email

@mflendrich
Copy link
Collaborator Author

mflendrich commented Apr 16, 2025 via email

@mflendrich
Copy link
Collaborator Author

mflendrich commented Apr 16, 2025 via email

@mflendrich
Copy link
Collaborator Author

/retest

@mflendrich
Copy link
Collaborator Author

mflendrich commented Apr 16, 2025 via email

@mflendrich
Copy link
Collaborator Author

mflendrich commented Apr 16, 2025 via email

@scylla-operator-bot scylla-operator-bot bot merged commit e6910a5 into scylladb:master Apr 17, 2025
13 checks passed
@mflendrich mflendrich added this to the v1.17.0 milestone May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rzetelskik rzetelskik

Assignees

@rzetelskik rzetelskik

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.17.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mflendrich @rzetelskik