-
Notifications
You must be signed in to change notification settings - Fork 65
🐛set preserveUnknownFields to false to make updating to crd v1 easier #293
🐛set preserveUnknownFields to false to make updating to crd v1 easier #293
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, great work!
/assign @chuckha
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nader-ziada, vincepri 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:
Approvers can indicate their approval by writing |
cd41977
to
db45f21
Compare
Makefile
Outdated
object:headerFile=./hack/boilerplate/boilerplate.generatego.txt \ | ||
paths=./api/... \ | ||
crd:preserveUnknownFields=false \ | ||
output:crd:dir=./config/crd/bases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should be missing lines 122 and 123 here, since it looks like the intent is to only generate the deepcopy code here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed them
Makefile
Outdated
rbac:roleName=manager-role \ | ||
webhook \ | ||
paths="./..." \ | ||
crd:preserveUnknownFields=false \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be changed up top where CRD_OPTIONS
is defined, also unsetting the trivialversions flag that is currently set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added the preserveUnknownFields option to CRD_OPTIONS
db45f21
to
b70ca10
Compare
@detiber addressed comments, ready for another look |
@nader-ziada did your changes get pushed? I'm not seeing the updates |
- also update controller tools to v0.2.5
b70ca10
to
91f096b
Compare
@detiber sorry, my bad, pushed now |
/lgtm |
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #290