-
Notifications
You must be signed in to change notification settings - Fork 0
feat(deps): replace yaml.v3 with goccy/go-yaml library #14
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
base: main
Are you sure you want to change the base?
Conversation
3a6418e to
f1a160e
Compare
|
Tests are failing cause the the YAML encoder does not return errors from the |
f1a160e to
2c737d2
Compare
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 pull request replaces the unmaintained yaml.v3 library with github.com/goccy/go-yaml while upgrading the minimum Go version and updating related dependencies and tests. Key changes include:
- Renaming and refactoring mock types and test cases to support the new yaml.InterfaceMarshaler and yaml.BytesMarshaler interfaces.
- Introducing encoder option support in the YAML renderer along with sequence indentation and auto integer conversion features.
- Upgrading Go version and dependencies in go.mod and updating CI workflow accordingly.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yaml_test.go | Updated tests and mock types to align with the new YAML library |
| yaml.go | Integrated new encoder options and improved encoder usage |
| render_test.go | Adjusted tests and message expectations to support new interface names |
| mise.toml | Removed obsolete tool versions |
| go.mod | Upgraded Go version and dependencies, including replacing yaml.v3 |
| .github/workflows/ci.yml | Updated CI workflow to reflect the new minimum Go version |
yaml.go
Outdated
| opts = append(opts, yaml.Indent(y.Indent)) | ||
| } | ||
| if len(y.EncodeOptions) > 0 { |
Copilot
AI
Jun 13, 2025
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.
Consider clarifying the interaction between the YAML.Indent field and the EncodeOptions. When both are provided, ensure that duplicate or conflicting indent options do not lead to unpredictable behavior.
| opts = append(opts, yaml.Indent(y.Indent)) | |
| } | |
| if len(y.EncodeOptions) > 0 { | |
| // Remove any existing yaml.Indent options from EncodeOptions | |
| filteredOptions := make([]yaml.EncodeOption, 0, len(y.EncodeOptions)) | |
| for _, opt := range y.EncodeOptions { | |
| if _, ok := opt.(yaml.IndentOption); !ok { | |
| filteredOptions = append(filteredOptions, opt) | |
| } | |
| } | |
| opts = append(opts, filteredOptions...) | |
| opts = append(opts, yaml.Indent(y.Indent)) | |
| } else if len(y.EncodeOptions) > 0 { |
| require ( | ||
| github.com/davecgh/go-spew v1.1.1 // indirect | ||
| github.com/pmezard/go-difflib v1.0.0 // indirect | ||
| gopkg.in/yaml.v3 v3.0.1 // indirect |
Copilot
AI
Jun 13, 2025
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 go.mod file still includes an indirect dependency on gopkg.in/yaml.v3 despite replacing it with github.com/goccy/go-yaml. Consider removing this dependency if it is no longer required.
| gopkg.in/yaml.v3 v3.0.1 // indirect | |
| # This line will be removed after running `go mod tidy`. |
|
bugbot run |
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.
Bug: YAML Render Method Causes Concurrency Issues
The YAML.Render method introduces two bugs. First, opts := YAMLDefaultEncoderOptions creates a slice sharing the underlying array with the global YAMLDefaultEncoderOptions. Appending to opts can modify the global variable if capacity allows, causing a race condition where default encoder options are unexpectedly altered for concurrent goroutines. Second, YAMLDefaultEncoderOptions already includes yaml.Indent(YAMLDefaultIndent). When y.Indent > 0, yaml.Indent(y.Indent) is appended to opts, resulting in two conflicting yaml.Indent options passed to the encoder. This changes the original behavior of y.Indent overriding the default and can cause unpredictable indentation.
yaml.go#L41-L49
Lines 41 to 49 in 2c737d2
| func (y *YAML) Render(w io.Writer, v any) error { | |
| opts := YAMLDefaultEncoderOptions | |
| if y.Indent > 0 { | |
| opts = append(opts, yaml.Indent(y.Indent)) | |
| } | |
| if len(y.EncodeOptions) > 0 { | |
| opts = append(opts, y.EncodeOptions...) | |
| } |
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $5.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
Replace yaml.v3 with goccy/go-yaml, as the former is now unmaintained. Also upgrade minimum Go version to 1.21.0 and update testify to v1.10.0. Add support for configuring encoder options in YAML renderer to provide more flexibility in YAML output formatting. Include new options for sequence indentation and automatic integer conversion. Implement support for both yaml.InterfaceMarshaler and yaml.BytesMarshaler interfaces with appropriate test cases. Rename mock implementation to clarify interface implementation.
2c737d2 to
6656062
Compare
Replace yaml.v3 with goccy/go-yaml, as the former is now unmaintained.
Also upgrade minimum Go version to 1.21.0 and update testify to v1.10.0.
Add support for configuring encoder options in YAML renderer to
provide more flexibility in YAML output formatting. Include new options
for sequence indentation and automatic integer conversion.
Implement support for both yaml.InterfaceMarshaler and
yaml.BytesMarshaler interfaces with appropriate test cases. Rename mock
implementation to clarify interface implementation.