-
Notifications
You must be signed in to change notification settings - Fork 69
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 gopkg.in/yaml.v3
#230
Conversation
Thanks for the PR and for bringing up the maintenance status of |
That would be preferable. |
@Juneezee, would you be up for reworking this PR to use |
At the time of making this commit, the package `github.com/ghodss/yaml` is no longer actively maintained. Signed-off-by: Eng Zer Jun <[email protected]>
github.com/ghodss/yaml
with sigs.k8s.io/yaml
github.com/ghodss/yaml
with gopkg.in/yaml.v3
Done! |
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 just ran some tests locally and it seems that -- unlike github.com/ghodss/yaml
-- gopkg.in/yaml.v3
does some case conversion by default:
package main
import (
orig "github.com/ghodss/yaml"
yamlv3 "gopkg.in/yaml.v3"
)
type NodeInfo struct {
ID uint64
Address string
Role int
}
func main() {
info := NodeInfo{ 5, "abc", 7 }
data, _ := orig.Marshal(&info)
println(string(data))
data, _ = yamlv3.Marshal(&info)
println(string(data))
}
$ go run
Address: abc
ID: 5
Role: 7
id: 5
address: abc
role: 7
Could you add yaml:
annotations on this declaration so that the replacement will be backwards-compatible? (We want each new version of go-dqlite to be able to read node store and info files written by a previous version.)
go-dqlite/internal/protocol/store.go
Lines 26 to 30 in a51f109
type NodeInfo struct { | |
ID uint64 | |
Address string | |
Role NodeRole | |
} |
Yup, I noticed the same too. The tests are failing with both
|
Unlike `github.com/ghodss/yaml`, `gopkg.in/yaml.v3` marshals using the field name lowercased as the default key [1]. [1]: https://pkg.go.dev/gopkg.in/yaml.v3#Marshal 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.
Thanks!
seems to hit a segfault on s390x https://launchpadlibrarian.net/655817390/buildlog_ubuntu-focal-s390x.go-dqlite_1.11.7+git5-g394129c~focal1_BUILDING.txt.gz
|
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.