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.

Sign up for GitHub

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

Jump to bottom

Check if both js-yaml and YAML libraries are still required #5435

Closed
antgamdia opened this issue Oct 6, 2022 · 0 comments · Fixed by #5627
Closed

Check if both js-yaml and YAML libraries are still required #5435

antgamdia opened this issue Oct 6, 2022 · 0 comments · Fixed by #5627
Assignees
@antgamdia
Labels
component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature
Milestone
Technical debt

Comments

@antgamdia
Copy link
Contributor

Summary
After #5412 is done, there are still two dependencies for handling YAML files, js-yaml and YAML. We should revisit the usage of each one to verify if they are both required.

Background and rationale
Less duplicated dependencies.

Description
Review the usage of js-yaml and YAML.

Acceptance criteria

  • The js-yaml and YAML dependencies have been analyzed
  • Unnecessary dependencies have been removed.

Additional context
Related to #4917
@antgamdia antgamdia added component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature labels Oct 6, 2022
@antgamdia antgamdia added this to the Technical debt milestone Oct 6, 2022
@antgamdia antgamdia mentioned this issue Oct 6, 2022
Closed
@ppbaena ppbaena added the next-iteration Issues to be discussed in planning session label Nov 4, 2022
@antgamdia antgamdia mentioned this issue Nov 10, 2022
Merged
absoludity added a commit that referenced this issue Nov 17, 2022
@antgamdia @absoludity
Remove js-yaml dependency (#5627)
4eb0f19 
### 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

- fixes #5435

### Additional information

N/A

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Co-authored-by: Michael Nelson <minelson@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@antgamdia antgamdia

Labels
component/ui Issue related to kubeapps UI kind/enhancement An issue that reports an enhancement for an implemented feature
Projects
Kubeapps
Archived in project
Milestone
Technical debt
Development

Successfully merging a pull request may close this issue.

Remove js-yaml dependency antgamdia/kubeapps
2 participants
@ppbaena @antgamdia