-
Notifications
You must be signed in to change notification settings - Fork 846
chore: Move to maintained yaml library #4227
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
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.
Pull Request Overview
This PR removes the gopkg.in/yaml.v2 dependency from the vendored dependencies. The change completely removes all yaml.v2 source files and updates the vendor modules list accordingly.
Key Changes
- Removed yaml.v2 entry from vendor/modules.txt
- Deleted all yaml.v2 source files from vendor/gopkg.in/yaml.v2/ directory
- Removed yaml.v2 README documentation
Reviewed Changes
Copilot reviewed 7 out of 26 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vendor/modules.txt | Removed yaml.v2 v2.4.0 dependency declaration |
| vendor/gopkg.in/yaml.v2/*.go | Deleted all yaml.v2 Go source files including core functionality |
| vendor/gopkg.in/yaml.v2/README.md | Removed yaml.v2 package documentation |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #4227 +/- ##
===========================================
- Coverage 54.49% 41.05% -13.45%
===========================================
Files 134 252 +118
Lines 12329 17796 +5467
===========================================
+ Hits 6719 7306 +587
- Misses 5116 9847 +4731
- Partials 494 643 +149
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JaydipGabani
left a comment
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.
@mrueg Thanks for the PR!
| "strings" | ||
|
|
||
| "gopkg.in/yaml.v3" | ||
| "go.yaml.in/yaml/v3" |
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.
@mrueg should we just move to v4? are there breaking changes from v3 - v4?
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 wanted to keep the change set minimal. v4 hasn't had an official release. latest is https://github.com/yaml/go-yaml/releases/tag/v4.0.0-rc.3
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.
Makes sense.
JaydipGabani
left a comment
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. @sozercan ptal
| "strings" | ||
|
|
||
| "gopkg.in/yaml.v3" | ||
| "go.yaml.in/yaml/v3" |
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.
Makes sense.
|
@mrueg can you resolve conflict? |
Rebased. |
|
do we need to vendor this? lgtm otherwise, thanks! |
cmd/gator/expand/expand.go
Outdated
| "github.com/open-policy-agent/gatekeeper/v3/pkg/gator/reader" | ||
| "github.com/spf13/cobra" | ||
| "gopkg.in/yaml.v2" // yaml.v3 inserts a space before '-', which is inconsistent with standard, kubernetes and kubebuilder format. yaml.v2 does not insert these spaces. | ||
| "go.yaml.in/yaml/v2" // yaml.v3 inserts a space before '-', which is inconsistent with standard, kubernetes and kubebuilder format. yaml.v2 does not insert these spaces. |
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.
can we get rid of this comment now?
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.
@mrueg can you address this.
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.
@mrueg ping on this.
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 the comment.
|
@mrueg plesae fix the conflicts. |
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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
|
@mrueg can you address the feedback above? |
|
@JaydipGabani which one? The one from CoPilot seems to be wrong, as it effectively reverts this change. |
|
Ohh nvm, I thought the earlier feedback was not addressed, we can dismiss copilot one. My appologies. |
Signed-off-by: Manuel Rüger <[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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
| "encoding/json" | ||
|
|
||
| "gopkg.in/yaml.v3" | ||
| "go.yaml.in/yaml/v3" |
Copilot
AI
Jan 5, 2026
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.
The import path "go.yaml.in/yaml/v3" appears to be incorrect. The standard maintained YAML library for Go uses the import path "gopkg.in/yaml.v3" (note the period before v3, not a slash). The repository at https://github.com/go-yaml/yaml documents that the correct import paths are:
- gopkg.in/yaml.v3 for v3
- gopkg.in/yaml.v2 for v2
Please verify that "go.yaml.in/yaml/v3" is a valid and maintained module. If the intention is to use the standard go-yaml library, the import should be "gopkg.in/yaml.v3" instead.
| "go.yaml.in/yaml/v3" | |
| "gopkg.in/yaml.v3" |
Signed-off-by: Manuel Rüger <[email protected]> Co-authored-by: Jaydip Gabani <[email protected]> Signed-off-by: Oliver Karstoft <[email protected]>
What this PR does / why we need it:
gopkg.in/yaml is unmaintained, this moves the dependency to a maintained yaml library https://github.com/yaml/go-yaml which is also used by https://github.com/kubernetes-sigs/yaml?tab=readme-ov-file#compatibility (which is used in gatekeeper as well)
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #
Special notes for your reviewer: