Discourage usage of nullable on API types #8488
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
sig/architecture
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: JoelSpeed The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
sig/contributor-experience
| For example, a map field marked with `+nullable` would accept either `foo: null` or `foo: {}`. | ||
|
|
||
| Usage of `+nullable` is discouraged as it introduces several issues: | ||
| - It is not compatible with json merge patching. |
There was a problem hiding this comment.
(from the merge patch RFC: "Null values in the merge patch are given special meaning to indicate the removal of existing values in the target.")
|
|
||
| Usage of `+nullable` is discouraged as it introduces several issues: | ||
| - It is not compatible with json merge patching. | ||
| - It is not compatible with generic protobuf. |
There was a problem hiding this comment.
| - It is not compatible with generic protobuf. | |
| - Explicit `null` values are not be persisted in proto serializations |
|
|
||
| Usage of `+nullable` is discouraged as it introduces several issues: | ||
| - It is not compatible with json merge patching. | ||
| - It is not compatible with generic protobuf. | ||
|
|
There was a problem hiding this comment.
I think it's also not compatible with server-side apply, but this should be verified (I have a vague memory of SSA using null values as a signal to remove a key, which would make a null value impossible to persist via SSA)
From conversation around #8486, we realised that the usage of nullable is problematic and would like to discourage new usage of it.