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

Migrate from ghodss/yaml to sigs.k8s.io/yaml #4179

Merged
merged 1 commit into from
Jan 31, 2022
Merged

Conversation

antgamdia
Copy link
Contributor

Description of the change

This PR just replaces ghodss/yaml with Kubernetes fork of the same library, sigs.k8s.io/yaml, as it was the most used along our codebase.

Benefits

Reduce same-purpose deps.

Possible drawbacks

I don't know we are ever relying on a very specific feature of the original dep whose behavior differs from the k8s-fork one. I can't think of it, but let's our CI decide.

Applicable issues

Additional information

N/A

Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

Not sure we need to use custom names for the imports? I'd do that only where it needs disambiguation (ie. if one file imported both for some reason), otherwise, I'd avoid naming k8syaml and goyaml when there's no ambiguity in the file.

@absoludity
Copy link
Contributor

Not sure we need to use custom names for the imports? I'd do that only where it needs disambiguation (ie. if one file imported both for some reason), otherwise, I'd avoid naming k8syaml and goyaml when there's no ambiguity in the file.

Oh, I should have read #4176 first, I'll reply there.

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
@antgamdia antgamdia changed the base branch from unifyImports-2 to main January 31, 2022 16:12
@antgamdia antgamdia merged commit c60cb00 into main Jan 31, 2022
@antgamdia antgamdia deleted the unifyImports-3.2 branch January 31, 2022 16:36
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

2 participants