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

Remove js-yaml dependency #5627

Merged
merged 5 commits into from
Nov 17, 2022
Merged

Conversation

antgamdia
Copy link
Contributor

Description of the change

After the revamping of the basic forms view, we left untouched the tech debt pointed in #5435: we were using two different libraries for the same purpose - dealing with YAML files.

This PR is removing js-yaml in favor of yaml, which offers a node representation of the parsed AST and makes, therefore, possible, dealing with comments in the file.

Benefits

A single library for dealing with yaml files.

Possible drawbacks

I've tried to preserve the same behavior, but, for instance, the quotation policies in js-yaml differ from the ones in yaml. Not a big deal, though.

Applicable issues

Additional information

N/A

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
@netlify
Copy link

netlify bot commented Nov 10, 2022

Deploy Preview for kubeapps-dev canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 3a2661a
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/6375b98c19c1530008183477

antgamdia and others added 2 commits November 10, 2022 15:15
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
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.

LGTM, just re-running CI after merging latest.

@absoludity absoludity merged commit 4eb0f19 into vmware-tanzu:main Nov 17, 2022
@antgamdia antgamdia deleted the 5435-removeYamlDep branch December 18, 2023 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check if both js-yaml and YAML libraries are still required
3 participants