knative / func

Knative Functions client API and CLI
Apache License 2.0
264 stars 135 forks source link

`func` does not retain user comments on `func.yaml` #603

Open jrangelramos opened 2 years ago

jrangelramos commented 2 years ago

Comments added on func.yaml by developers are lost every time a func command that writes to this file is executed.

As an example, I added a label (or env var), then I manually changed the func.yaml with a comment to describe that label (or env).

labels:
# My label
- key: mylabel
  value: myvalue

After adding another label thru func config, or after running a func build my comment get removed from the yaml file.

labels:
- key: mylabel
  value: myvalue
lance commented 2 years ago

I'm not sure if this is really going to be possible. See: https://github.com/knative-sandbox/kn-plugin-func/issues/598#issuecomment-948896011

jrangelramos commented 2 years ago

I think it would be a great addition if user could self document func.yaml. In case that not really possible, what about (if possible) to add some generic comment on top of func yaml explaining the file is self maintained and any comments would be deleted? It is worth?

examplefunc.yaml

# This file is self maintained by func. Any comments made here by user might be lost.
name: myfunc
namespace: ""
runtime: node
...
zroubalik commented 2 years ago

We would probably need a custom YAML de/serializer?

lance commented 2 years ago

We would probably need a custom YAML de/serializer?

I think so, which seems to be a lot of work for this small benefit. Maybe we should instead, normalize the README.md files that are written from the default templates, and explicitly note the role of func.yaml.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

zroubalik commented 2 years ago

/remove-lifecycle stale

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

zroubalik commented 2 years ago

/remove-lifecycle stale

lance commented 1 year ago

Adding comments from @rhuss about this in https://github.com/knative-sandbox/kn-plugin-func/issues/598

It would be super awesome if some examples for how to customize func.yaml could be added as comments. Currently I want to customize the envs property but have no clue what to add as array. Of course, I can look it up (and will do), but having this directly in the gerated func.yaml as an example (in a yaml comment), that would be great.

Closing #598 as duplicate of this, assuming that we will add comments to func.yaml if possible during the implementation of this feature.

lance commented 1 year ago

/kind enhancement

grafvonb commented 1 year ago

It seems that yaml is the wrong format if we want to preserve comments and formatting as explained here: https://stackoverflow.com/questions/62062329/how-to-parse-general-yaml-in-golang-with-comments-preserved

lkingland commented 1 year ago

It seems that yaml is the wrong format

💯

@lance perhaps we should close this issue? We'll of course need a constant campaign of directing users to the func cli for imperative mutations of their Function's metadata, and to their source code itself for declarative. In addition, maybe now is the time to re-start the conversation about the in-function-instance API for accomplishing many of the tasks people who are familiar with kubectl or kn may first think to look in a .yaml for out of habit; and start gently moving towards source code declarations

lance commented 1 year ago

@lance perhaps we should close this issue?

I'm not sure. It's tempting, no doubt, since this issue has been open for more than a year. I've felt from the beginning that this issue would not be easily solved if we stick with yaml (which I think we should).

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

lance commented 1 year ago

I'm going to go ahead and close this as wont-fix. Please reopen if you feel strongly against this.