-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
refactor: replace github.com/ghodss/yaml
with sigs.k8s.io/yaml
#6935
refactor: replace github.com/ghodss/yaml
with sigs.k8s.io/yaml
#6935
Conversation
At the time of making this commit, the package `github.com/ghodss/yaml` is no longer actively maintained. `sigs.k8s.io/yaml` is a permanent fork of `ghodss/yaml` and is actively maintained by Kubernetes SIG. Signed-off-by: Eng Zer Jun <[email protected]>
Hi @Juneezee. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/ok-to-test |
In `sigs.k8s.io/yaml`, yaml.UnmarshalStrict enables DisallowUnknownFields automatically. Fixes: 666feef ("installconfig: Move fatal error to warn for strict marshal") Signed-off-by: Eng Zer Jun <[email protected]>
Is there some place left still using |
@r4f4 One of our dependency https://github.com/openshift/cluster-api-provider-libvirt is still using
|
So maybe we could use a replace directive to take care of that until cluster-api-provider-libvirt moves away from it? replace github.com/ghodss/yaml => sigs.k8s.io/yaml |
@r4f4 Go doesn't seem to allow this
Is it okay if I go open a PR in https://github.com/openshift/cluster-api-provider-libvirt to replace |
Oh I see.
Yep, I think that's the proper way to solve this problem. |
Marking this PR as draft until openshift/cluster-api-provider-libvirt#252 is merged. |
…aml` Reference: openshift/cluster-api-provider-libvirt#252 Signed-off-by: Eng Zer Jun <[email protected]>
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: r4f4 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 |
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.
/lgtm
@Juneezee: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
The package
github.com/ghodss/yaml
is no longer actively maintained. See discussion in ghodss/yaml#75 and ghodss/yaml#80.sigs.k8s.io/yaml
is a permanent fork ofgithub.com/ghodss/yaml
.The notable change is that
github.com/ghodss/yaml
usesgopkg.in/yaml.v2 v2.2.2
, butsigs.k8s.io/yaml
usesgopkg.in/yaml.v2 v2.4.0
. Changes can be seen here v2.2.2...v2.4.0, mostly bug fixes.