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

Consider only the last group of comments starting with '# --'. #99

Merged
merged 2 commits into from
Jan 18, 2022

Conversation

bmcustodio
Copy link
Contributor

@bmcustodio bmcustodio commented May 20, 2021

Attempts to fix #96. It seems to work for a complex chart I tested with on which the same issue was being observed. I've also added a couple tests.

Signed-off-by: Bruno Miguel Custódio <[email protected]>
errordeveloper pushed a commit to cilium/cilium that referenced this pull request Jun 25, 2021
'helm-docs' has a bug which causes it to include comments belonging to
previously-appearing but commented-out fields. A fix has been proposed
in norwoodj/helm-docs#99, but hasn't been
reviewed yet. While said PR doesn't get merged it's preferable to switch
to a fork containing the fix so we can have a proper description for our
Helm chart fields.

Signed-off-by: Bruno Miguel Custódio <[email protected]>
errordeveloper pushed a commit to cilium/cilium that referenced this pull request Jun 25, 2021
'helm-docs' has a bug which causes it to include comments belonging to
previously-appearing but commented-out fields. A fix has been proposed
in norwoodj/helm-docs#99, but hasn't been
reviewed yet. While said PR doesn't get merged it's preferable to switch
to a fork containing the fix so we can have a proper description for our
Helm chart fields.

[ upstream commit 8d4f1ea ]

Signed-off-by: Bruno Miguel Custódio <[email protected]>
errordeveloper pushed a commit to cilium/cilium that referenced this pull request Jun 25, 2021
'helm-docs' has a bug which causes it to include comments belonging to
previously-appearing but commented-out fields. A fix has been proposed
in norwoodj/helm-docs#99, but hasn't been
reviewed yet. While said PR doesn't get merged it's preferable to switch
to a fork containing the fix so we can have a proper description for our
Helm chart fields.

[ upstream commit 8d4f1ea ]

Signed-off-by: Bruno Miguel Custódio <[email protected]>
@bmcustodio
Copy link
Contributor Author

@norwoodj do you have any plans for merging this in the near future?

@pchaigno
Copy link

pchaigno commented Jan 3, 2022

@skang0601 Did you get a chance to look into this? It fixes a bug we're hitting at cilium/cilium.

vincentmli added a commit to vincentmli/cilium that referenced this pull request Sep 21, 2022
…ence

helm options that are commented out in install/kubernetes/cilium/values.yaml.tmpl
are missing documentation in Documentation/helm-values.rst and in helm reference
this is reported in a few GH issues, for example:
cilium#21107

This appears to be known helm-doc bug and the bug has not been resolved, a related
PR norwoodj/helm-docs#99 that seems has not completely resloved
the issue.

before the helm-doc bug is root caused, uncomment these helm options as workaround.

Since Documentation/helm-values.rst install/kubernetes/cilium/README.md are auto
generated, it is difficult to avoid rebase/squash conflicts in local branch and
upstream master branch, so introduce the helm option in smaller changes so it easy
to review and rebase/squash and merge.

Fixes: cilium#21334 cilium#21107

Signed-off-by: Vincent Li <[email protected]>
pchaigno pushed a commit to cilium/cilium that referenced this pull request Sep 21, 2022
…ence

helm options that are commented out in install/kubernetes/cilium/values.yaml.tmpl
are missing documentation in Documentation/helm-values.rst and in helm reference
this is reported in a few GH issues, for example:
#21107

This appears to be known helm-doc bug and the bug has not been resolved, a related
PR norwoodj/helm-docs#99 that seems has not completely resloved
the issue.

before the helm-doc bug is root caused, uncomment these helm options as workaround.

Since Documentation/helm-values.rst install/kubernetes/cilium/README.md are auto
generated, it is difficult to avoid rebase/squash conflicts in local branch and
upstream master branch, so introduce the helm option in smaller changes so it easy
to review and rebase/squash and merge.

Fixes: #21334 #21107

Signed-off-by: Vincent Li <[email protected]>
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.

The description includes description of previous commented values
3 participants