Skip to content

🐛 Allow setting PreserveUnknownFields at both type and field level#689

Merged
k8s-ci-robot merged 1 commit into
kubernetes-sigs:masterfrom
eddycharly:master
Jun 13, 2022
Merged

🐛 Allow setting PreserveUnknownFields at both type and field level#689
k8s-ci-robot merged 1 commit into
kubernetes-sigs:masterfrom
eddycharly:master

Conversation

@eddycharly

@eddycharly eddycharly commented Jun 2, 2022

Copy link
Copy Markdown
Contributor

This PR allows setting PreserveUnknownFields at both type and field level.

Fixes #688

@eddycharly
fix: allow setting PreserveUnknownFields at both type and field level
1df4fab 
Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 2, 2022
@k8s-ci-robot

k8s-ci-robot commented Jun 2, 2022

Copy link
Copy Markdown
Contributor

Hi @eddycharly. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 2, 2022
@k8s-ci-robot k8s-ci-robot requested review from droot and pwittrock June 2, 2022 12:17
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 2, 2022
@eddycharly eddycharly mentioned this pull request Jun 2, 2022
Closed
@FillZpp

FillZpp commented Jun 6, 2022

Copy link
Copy Markdown
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 6, 2022
@eddycharly eddycharly marked this pull request as ready for review June 6, 2022 07:21
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 6, 2022
@k8s-ci-robot k8s-ci-robot requested a review from mengqiy June 6, 2022 07:21
@eddycharly

eddycharly commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

/assign @pwittrock

@eddycharly

eddycharly commented Jun 8, 2022

Copy link
Copy Markdown
Contributor Author

@FillZpp did you have time to look at this PR ?

@FillZpp

FillZpp commented Jun 9, 2022

Copy link
Copy Markdown
Contributor

@FillZpp did you have time to look at this PR ?

Sure, it looks good to me. You may need another approver to review it.

/lgtm
/cc @alvaroaleman

@k8s-ci-robot k8s-ci-robot requested a review from alvaroaleman June 9, 2022 02:49
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 9, 2022
@FillZpp FillZpp mentioned this pull request Jun 13, 2022
Merged
alvaroaleman
alvaroaleman approved these changes Jun 13, 2022
View reviewed changes

@alvaroaleman alvaroaleman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm
/approve
thanks!

@k8s-ci-robot

k8s-ci-robot commented Jun 13, 2022

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, eddycharly

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2022
@k8s-ci-robot k8s-ci-robot merged commit d97fa93 into kubernetes-sigs:master Jun 13, 2022
@eddycharly eddycharly mentioned this pull request Jul 2, 2022
Closed
@FillZpp

FillZpp commented Jul 4, 2022

Copy link
Copy Markdown
Contributor

/cherrypick release-0.9

@k8s-infra-cherrypick-robot

k8s-infra-cherrypick-robot commented Jul 4, 2022

Copy link
Copy Markdown

@FillZpp: #689 failed to apply on top of branch "release-0.9":

Applying: fix: allow setting PreserveUnknownFields at both type and field level
Using index info to reconstruct a base tree...
M	pkg/crd/flatten.go
M	pkg/crd/testdata/cronjob_types.go
M	pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
Falling back to patching base and 3-way merge...
Auto-merging pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml
Auto-merging pkg/crd/testdata/cronjob_types.go
Auto-merging pkg/crd/flatten.go
CONFLICT (content): Merge conflict in pkg/crd/flatten.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix: allow setting PreserveUnknownFields at both type and field level
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Details

In response to this:

/cherrypick release-0.9

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@FillZpp

FillZpp commented Jul 4, 2022

Copy link
Copy Markdown
Contributor

@eddycharly You may have to do this cherry-pick manually.

eddycharly pushed a commit to eddycharly/controller-tools that referenced this pull request Jul 4, 2022
@k8s-ci-robot @eddycharly
Merge pull request kubernetes-sigs#689 from eddycharly/master
65ed27c 
🐛 Allow setting PreserveUnknownFields at both type and field level

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>
This was referenced Jul 4, 2022
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alvaroaleman alvaroaleman alvaroaleman approved these changes

@droot droot Awaiting requested review from droot

@pwittrock pwittrock Awaiting requested review from pwittrock

@mengqiy mengqiy Awaiting requested review from mengqiy

Assignees

@alvaroaleman alvaroaleman

@FillZpp FillZpp

@pwittrock pwittrock

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Setting PreserverUnknownFields marker on both type and field generates an invalid schema

6 participants

@eddycharly @k8s-ci-robot @FillZpp @k8s-infra-cherrypick-robot @alvaroaleman @pwittrock