Hackathon test api change for int32 to int64#26
Hackathon test api change for int32 to int64#26richabanker wants to merge 1 commit intohackathonfrom
Conversation
richabanker
force-pushed
the
hackathon-test-api-review-2
branch
from
4e387d8 to
a2fb2f0
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // The number of pods that have labels matching the labels of the pod template of the replication controller. | ||
| // +optional | ||
| FullyLabeledReplicas int32 | ||
| FullyLabeledReplicas int64 |
There was a problem hiding this comment.
Changing FullyLabeledReplicas from int32 to int64 seems unusual for a count of replicas. While not inherently wrong, consider the implications. Does this RC ever realistically manage more than 2,147,483,647 replicas? If not, int32 is more memory-efficient and aligns with other replica counts in the API. If there's a specific reason for expecting a higher number of replicas, then int64 is appropriate. Please clarify the reason for this change. Without a clear justification, it seems safer to keep it as int32.
richabanker
force-pushed
the
hackathon
branch
6 times, most recently
from
4c04c6a to
a311a42
Compare
richabanker
force-pushed
the
hackathon-test-api-review-2
branch
from
a2fb2f0 to
455f927
Compare
richabanker
force-pushed
the
hackathon
branch
from
e330f26 to
29df677
Compare
richabanker
force-pushed
the
hackathon-test-api-review-2
branch
from
455f927 to
cff3adc
Compare
richabanker
force-pushed
the
hackathon
branch
from
4f7dbdb to
6b1984c
Compare
richabanker
force-pushed
the
hackathon-test-api-review-2
branch
from
cff3adc to
f033362
Compare
richabanker
force-pushed
the
hackathon
branch
from
5fd75a7 to
a933833
Compare
richabanker
force-pushed
the
hackathon-test-api-review-2
branch
from
f033362 to
6313ba4
Compare
| @@ -4361,7 +4361,7 @@ type ReplicationControllerStatus struct { | |||
|
|
|||
| // The number of pods that have labels matching the labels of the pod template of the replication controller. | |||
| // +optional | |||
There was a problem hiding this comment.
It is recommended that all public integer fields MUST use the Go int32 or Go int64 types, not int. The change from int32 to int64 may be necessary if replicas can be greater than max int32. If it is likely not to be greater than max int32, suggest not changing it since it is more narrow.
richabanker
force-pushed
the
hackathon
branch
from
039d256 to
99ea87f
Compare
| @@ -4361,7 +4361,7 @@ type ReplicationControllerStatus struct { | |||
|
|
|||
| // The number of pods that have labels matching the labels of the pod template of the replication controller. | |||
| // +optional | |||
There was a problem hiding this comment.
Consider using int32 for FullyLabeledReplicas unless there's a specific reason to expect it to exceed the maximum value of int32. Using int32 is generally preferred for integers unless int64 is necessary.
richabanker
force-pushed
the
hackathon-test-api-review-2
branch
from
6313ba4 to
cd88116
Compare
| @@ -4361,7 +4361,7 @@ type ReplicationControllerStatus struct { | |||
|
|
|||
| // The number of pods that have labels matching the labels of the pod template of the replication controller. | |||
| // +optional | |||
There was a problem hiding this comment.
It's generally preferable to use int32 unless int64 is specifically required. Check if int32 is sufficient for the range of FullyLabeledReplicas, to avoid potential compatibility issues with Javascript number representation.
richabanker
force-pushed
the
hackathon
branch
2 times, most recently
from
2c94705 to
a535d8c
Compare
1 participant
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: