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

docs(redis-ha): Introduce helm-docs for values documentation #280

Conversation

mkilchhofer
Copy link
Contributor

@mkilchhofer mkilchhofer commented May 22, 2024

What this PR does / why we need it:

Manually ensuring proper documentation for all parameters is almost impossible and also in this redis-ha helm chart a plenty of variables were undocumented.

The helper utility helm-docs has a wide adoption inside CNCF projects.

Which issue this PR fixes

none

Special notes for your reviewer:

I already templated the chart before my change and after my change

$ helm template . > rendered-master.yaml

$ git co -
Switched to branch 'feature/redis-ha_introduce_helm-docs'
Your branch is up to date with 'origin/feature/redis-ha_introduce_helm-docs'.

$ helm template . > rendered-feature_redis-ha_introduce_helm-docs.yaml

$ diff -u rendered-master.yaml rendered-feature_redis-ha_introduce_helm-docs.yaml
$ echo $?
0

This should verify that we do not introduce a regression with this docs change.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md
  • Title of the PR starts with chart name (e.g. [stable/mychartname])

@mkilchhofer mkilchhofer force-pushed the feature/redis-ha_introduce_helm-docs branch from 560d858 to 39658c2 Compare May 22, 2024 20:17
@mkilchhofer mkilchhofer marked this pull request as ready for review May 22, 2024 20:22
@DandyDeveloper
Copy link
Owner

Helm docs is new to me! Let me go through this quickly so I can approve knowing I'm up to speed!

@mkilchhofer mkilchhofer force-pushed the feature/redis-ha_introduce_helm-docs branch from 39658c2 to b598c55 Compare May 26, 2024 15:01
@mkilchhofer
Copy link
Contributor Author

@DandyDeveloper friendly reminder

@mkilchhofer
Copy link
Contributor Author

@DandyDeveloper another friendly ping

Are You There GIF

@DandyDeveloper
Copy link
Owner

I am here! Just neglected my duties this past couple of months. Sorry!

DandyDeveloper
DandyDeveloper previously approved these changes Nov 5, 2024
@DandyDeveloper DandyDeveloper merged commit 5600960 into DandyDeveloper:master Nov 6, 2024
2 checks passed
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.

2 participants