Skip to content
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

chore: Replace ghodss/yaml with sigs.k8s.io/yaml #6195

Merged
merged 2 commits into from Aug 30, 2023

Conversation

mrueg
Copy link
Contributor

@mrueg mrueg commented Aug 29, 2023

Why the changes in this PR are needed?

ghodss/yaml hasn't seen any commits since 2019 (the last one was a change in README), sigs.k8s.io/yaml is a more recent and more active fork supported by the kubernetes community.
https://github.com/kubernetes-sigs/yaml

What are the changes in this PR?

Migrate dependency from ghodss/yaml to sigs.k8s.io/yaml

Notes to assist PR review:

Further comments:

Here's a similar change that got included in gatekeeper already:
open-policy-agent/gatekeeper#2697

This is a fork that is maintained by the kubernetes community.

https://github.com/kubernetes-sigs/yaml
Signed-off-by: Manuel Rüger <manuel@rueg.eu>
@anderseknert
Copy link
Member

anderseknert commented Aug 29, 2023

Hi there! And thanks. #5754 provides some context for why this hasn’t been considered too important in the past. Do you know if the issue described there is solved now? I’m on mobile so I can’t easily see until later :)

@mrueg
Copy link
Contributor Author

mrueg commented Aug 29, 2023

@anderseknert 👋 thanks for the context, I wasn't aware that there's an issue already! I'll take a look tomorrow and see if this change is able to fix the issue as well.

@anderseknert
Copy link
Member

Just to be clear, I didn’t mean we shouldn’t merge this if not. But whatever we use would be temporary until we have a something addressing those issues.

@mrueg
Copy link
Contributor Author

mrueg commented Aug 30, 2023

The issue #5754 is not fixed by this change but likely will be if kubernetes-sigs/yaml#76 is merged into the upstream repository.

Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

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

LGTM

@johanfylling johanfylling merged commit 9cab6c9 into open-policy-agent:main Aug 30, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants