Skip to content

Conversation

@dmolik
Copy link
Collaborator

@dmolik dmolik commented Oct 5, 2025

Signed-off-by: Dan Molik [email protected]

Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

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

Sorry for late review.. looks good. Added some comments to chat about


// Override resource requirements for each pod
// +optional
Resources corev1.ResourceRequirements `json:"resources,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't a ptr compared to the Affinity option that also is optional.
Is this to be handled differently then checking it for nil?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No I'll clean it up

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There seems to be an issue with Deepcopy, the linter throws an error if we try to make resource requirements a pointer
https://github.com/valkey-io/valkey-operator/actions/runs/18779683212/job/53582458472?pr=5#step:4:34

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.

2 participants