-
Notifications
You must be signed in to change notification settings - Fork 385
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
Migrate from ghodss/yaml to gopkg.in/yaml.v3 #1818
Conversation
This requires us to _manually_ deal with the bytes-as-base64-strings format. Signed-off-by: Miloslav Trmač <[email protected]>
This has insufficient unit test coverage... Signed-off-by: Miloslav Trmač <[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.
LGTM
Nice LGTM. |
Looks like vendoring is still pulling in
|
@rhatdan, it's gone. The |
It is listed in go.mod above? |
How could I have missed that, apologies.
|
|
Yes; the PR description mentions this dependency. We are getting rid of |
This attempts to remove a dependency on yaml.v2 which we, sort of, control. This parallels containers/skopeo#1885 .
At the very least this will allow removing
ghodss/yaml
from Skopeo (but not from Podman, because Podman uses this package directly). That’s at least a bit of code.Note that the whole of c/image will continue to depend on
yaml.v2
, via a Swagger-generated Rekor client, depending on go-openapi/runtime#245 .Tested, so far, only using unit tests; that suggests that the migration from
ghodss/yaml
might be fairly tricky. Theopenshift
case is one where we would be quite unlikely to notice breakage in practice, but it has fairly good unit tests. Theregistries.d
case has worse unit tests, but simpler data types and probably better integration test coverage.