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

Keep comments in chartfile.yaml? #955

Closed
strowi opened this issue Nov 15, 2023 · 2 comments
Closed

Keep comments in chartfile.yaml? #955

strowi opened this issue Nov 15, 2023 · 2 comments

Comments

@strowi
Copy link
Contributor

strowi commented Nov 15, 2023

Hi,

the commands tk tool charts (add-repo|add update the chartfile and remove ALL comments. This might not be desired or expected.
Would it be possible to change the command to keep comments in the file?

Further explanation: we are using renovate-bot to manage almost all updates in our code, including tanka/charts. Therefore we added a regex to be able to use comments in the chartfile.yaml to detect and update:

requires:
  - chart: renovate/whitesource-renovate
    # renovate: datasource=helm depName=whitesource-renovate registryUrl=https://mend.github.io/renovate-on-prem versioning=helm
    version: 4.1.0
....

But these useful comments would be removed on add/add-repo..

@SimKev2
Copy link
Contributor

SimKev2 commented Apr 16, 2024

Hey @strowi thanks for this issue, I was taking a look as I also was running into this issue when updating our chart versions. Unfortunately it looks like the updates required to support this would be a major rewrite and make the code more difficult to work with.

First step is using the go-yaml.v3 subpackage of sigs.k8s.io/yaml since go-yaml.v2 has no support for when loading the file. After that it would require a solution that lets us use the Node type as an intermediary representation since that is the object that preserves comments https://github.com/go-yaml/yaml/blob/v3/yaml.go#L344-L347 . That would involve either updating every struct we have to contain the Node information as well as every reference to the values to work with that new representation, or changing all the functions to work on a raw Node rather than the representative go structs. There may also be a way to store the nodes for some post-processing merge but that would involve writing our own Node-transform functions.

That level of effort and change to the ergonomics of the codebase don't seem worth it at this time to support comments sticking around.

@strowi
Copy link
Contributor Author

strowi commented Apr 16, 2024

@SimKev2 totally understandable, thank you for looking into it and the explanation!

@strowi strowi closed this as completed Apr 16, 2024
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

No branches or pull requests

2 participants