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

Upgrade helm-docs #1784

Closed
jmeridth opened this issue Jan 20, 2023 · 7 comments
Closed

Upgrade helm-docs #1784

jmeridth opened this issue Jan 20, 2023 · 7 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers no-issue-activity size/S

Comments

@jmeridth
Copy link
Member

Is your feature request related to a problem?

no

Related helm chart

argo-cd, argo-events, argo-rollouts, argo-workflows, argocd-image-updater, argocd-apps

Describe the solution you'd like

Upgrade the scripts/helm-docs.sh file to use the latest jnorwood/helm-docs container image (currently 1.11.0). The current issue is that comments have to be reformatted in our README.md.gotmpl files so that they don't show up in the generated README.md files. That is mentioned here in its README.

Describe alternatives you've considered

not upgrading

Additional context

Was going to be a part of #1782 but ended up being beyond the scope of the PR.

@jmeridth jmeridth added enhancement New feature or request good first issue Good for newcomers size/S documentation Improvements or additions to documentation labels Jan 20, 2023
@jmeridth jmeridth added documentation Improvements or additions to documentation enhancement New feature or request size/S and removed enhancement New feature or request size/S documentation Improvements or additions to documentation labels Jan 20, 2023
@pdrastil
Copy link
Member

Not possible to use ## - ref norwoodj/helm-docs#148

@jmeridth
Copy link
Member Author

jmeridth commented Feb 18, 2023

@pdrastil yep. That's mentioned above. From the helm-docs README

New-style comments are much the same as
the old-style comments, except that while old comments
for a field could appear anywhere in the file,
new-style comments must appear on the line(s)
immediately preceding the field being documented.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@jmeridth
Copy link
Member Author

Not stale

@jdvgh
Copy link
Contributor

jdvgh commented May 11, 2023

I had a look into the code of helm-docs but apparently comments on the next lines are just concatenated with a space:
https://github.com/norwoodj/helm-docs/blob/7717a245cecabec154eccc0e9f7dbc43fb514754/pkg/helm/comment.go#L78-L80

See also here:
https://github.com/norwoodj/helm-docs/blob/master/README.md?plain=1#L252-L253

Note that comments can continue on the next line. In that case leave out the double dash, and the lines will simply be appended with a space in-between, as in the controller.replicas field in the example above

If they were at least combined with a newline, we would be able to parse \n# as a comment. However, with spaces this is not possible in my opinion (as # could also be used inside a comment, and then we would not "catch" the first comment started with ## ).

I tinkered a little bit with e.g. the clusterCredentials in the argo-cd values:

values.yaml

configs:
  # -- Provide one or multiple [external cluster credentials]
  # @default -- `[]` (See [values.yaml])
  ## Ref:
  ## - https://argo-cd.readthedocs.io/en/stable/operator-manual/declarative-setup/#clusters
  ## - https://argo-cd.readthedocs.io/en/stable/operator-manual/security/#external-cluster-credentials
  ## - https://argo-cd.readthedocs.io/en/stable/user-guide/projects/#project-scoped-repositories-and-clusters
  clusterCredentials: []

README.md.gotmpl

{{- range .Values }}
  {{- if hasPrefix "configs.clusterCredentials" .Key }}
    {{-  splitList " #" .AutoDescription }}
  {{- end }}
{{- end }}

result of running ./scripts/helm-docs.sh in README.md

5

So we got all 5 lines, but I don't know if this is stable enough to really implement - there could be quite a few edge cases; at least in contrast to just continue using the old version.

For reference, when just printing the AutoDescription we get the following:
README.md.gotmpl

{{- range .Values }}
  {{- if hasPrefix "configs.clusterCredentials" .Key }}
    {{-  .AutoDescription }}
  {{- end }}
{{- end }}

result of running ./scripts/helm-docs.sh in README.md

Provide one or multiple [external cluster credentials] # Ref: # - https://argo-cd.readthedocs.io/en/stable/operator-manual/declarative-setup/#clusters # - https://argo-cd.readthedocs.io/en/stable/operator-manual/security/#external-cluster-credentials # - https://argo-cd.readthedocs.io/en/stable/user-guide/projects/#project-scoped-repositories-and-clusters

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2023
@jmeridth jmeridth reopened this Aug 14, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers no-issue-activity size/S
Projects
None yet
Development

No branches or pull requests

3 participants