-
Notifications
You must be signed in to change notification settings - Fork 253
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.
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
We need to remove omitempty
from all values which aren't struct pointers
#1850
Comments
@kubernetes-sigs/cluster-api-provider-openstack-maintainers please add the |
Just to clarify, the statement applies only to structs. Non-struct, non-pointer fields may have omitempty, but you need to be wary of the unstructured vs structured serialisation of fields. For structs, the json marshaller cannot determine if a struct is at its zero value, therefore, it struct fields must be pointers to allow it to omit them. For a string, you can use a pointer or not, as the json marshaller will omit the The gotcha here is that, if you have a non-pointer field, some users may set the value to To avoid that, all fields that are optional are recommended, by the Kube API conventions, to have a |
Will be addressed by #1897 |
/assign mdbooth |
I promised to get back to this with an opinion once I had a chance to try to properly understand it. Now I have done that, and I agree that we should do this. 👍 |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues. This bot triages un-triaged issues according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
@mdbooth and team did this already. |
Originally posted by @mdbooth in #1729 (comment)
After offline discussion with @JoelSpeed about this, I think I understand a new API principal:
omitempty
is only for struct pointers. IOW we're massively over-using it.I'd like this to be another v1beta1 cleanup: remove omitempty from all values which aren't struct pointers. Unfortunately this may mean turning some structs (back) into pointers if we want to ensure they're omitted, which I hate because Go makes struct pointers in large data structures a syntactic minefield1.
Footnotes
Because it doesn't require a
->
operator like more civilised pointer languages, it's impossible to visually distinguish a pointer dereference from regular access of a struct field, which means it's easy to miss that you need a null check. Even worse, turning a struct into a struct pointer won't break compilation of existing references, meaning they will all have to be checked manually when making this change. ↩The text was updated successfully, but these errors were encountered: