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

Add Helm Chart configuration table reference to installation guide #1452

Merged
merged 3 commits into from
Jan 10, 2024

Conversation

ADubhlaoich
Copy link
Contributor

@ADubhlaoich ADubhlaoich commented Jan 8, 2024

Proposed changes

This commit adds a prominent section to the documentation's Helm installation guide to direct a reader to the Helm README which has all of its configurable parameters.

This information is being referenced instead of duplicated to avoid drift, which has the side-effect of making the Helm chart itself a single source of truth.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

This commit adds a prominent section to the documentation's Helm
installation guide to direct a reader to the Helm README which has all
of its configurable parameters.

This information is being referenced instead of duplicated to avoid
drift, which has the side-effect of making the Helm chart itself a
single source of truth.
@ADubhlaoich ADubhlaoich requested review from a team as code owners January 8, 2024 17:05
@ADubhlaoich ADubhlaoich self-assigned this Jan 8, 2024
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jan 8, 2024
@ADubhlaoich
Copy link
Contributor Author

As a point of possible change/discussion: it links to the current release branch/tag for the Helm Chart README.

I intend to create a cherry-pick to the release branch after this has been created: this means that the link is valid to the live release, though I'm unsure if the release process has a version bumping mechanism like NGINX Ingress Controller does.

We could link to the Helm website where the README is displayed instead.

@sjberman
Copy link
Contributor

sjberman commented Jan 8, 2024

Right now in our main branch we just link to the main branch for all the things. Then in the release branch, we manually update those links to point to the release branch.

There's definitely room for improvement with this process, ideally some sort of automation, but that hasn't been figured out yet.

@ADubhlaoich
Copy link
Contributor Author

ADubhlaoich commented Jan 8, 2024

@sjberman Will I change it to target main for the time being, then?

I can probably dig up the script NGINX Ingress Controller used for the bump (It's now a GitHub Action IIRC) if that's of any use for future reference.

@sjberman
Copy link
Contributor

sjberman commented Jan 8, 2024

Per that last point, would you be able to update the release process doc, specifically point 8.7 to say "Modify any docs links that refer to main to instead refer to vX.Y.Z or release-x-y, whichever is applicable."

@sjberman
Copy link
Contributor

sjberman commented Jan 8, 2024

@sjberman Will I change it to target main for the time being, then?

I would say yes, for now.

@ADubhlaoich
Copy link
Contributor Author

Changes requested made.

@ciarams87 ciarams87 merged commit 9b62bc5 into nginxinc:main Jan 10, 2024
27 checks passed
@ADubhlaoich ADubhlaoich deleted the docs/add-helm-params-reference branch January 10, 2024 11:13
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
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants