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: add a reference of helm values #16238

Merged
merged 1 commit into from Jun 7, 2021
Merged

docs: add a reference of helm values #16238

merged 1 commit into from Jun 7, 2021

Conversation

bmcustodio
Copy link
Contributor

@bmcustodio bmcustodio commented May 20, 2021

Adds a reference of all Helm values that can be set on the chart to the documentation.

docs: add a reference of helm values

@bmcustodio bmcustodio requested review from a team as code owners May 20, 2021 09:35
@bmcustodio bmcustodio requested review from a team, qmonnet and kkourt May 20, 2021 09:35
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 20, 2021
@bmcustodio bmcustodio added area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. backport/1.10 labels May 20, 2021
@aanm aanm added needs-backport/1.8 release-note/misc This PR makes changes that have no direct user impact. and removed backport/1.10 labels May 20, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 20, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.11 May 20, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.8 May 20, 2021
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice additions, but some items need some fix up.

Please let's make sure we only add the minimal set of entries required to the spelling list. A few of them are unnecessary, some of them are superseding the equivalent entries with capital letters, and for some I just don't understand how they ended up in the list.

Another important point - What does the result look like on your setup? The generated table is very wide in my browser, and anything past the two or three first columns needs a lot of scrolling, making the table unreadable in practice. Have you tried alternative ways to render the values, or at least looking at table options to see if there's a way to wrap the content?

Documentation/spelling_wordlist.txt Outdated Show resolved Hide resolved
Documentation/spelling_wordlist.txt Outdated Show resolved Hide resolved
Documentation/spelling_wordlist.txt Outdated Show resolved Hide resolved
Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/.gitignore Outdated Show resolved Hide resolved
Documentation/spelling_wordlist.txt Outdated Show resolved Hide resolved
Documentation/spelling_wordlist.txt Show resolved Hide resolved
Documentation/spelling_wordlist.txt Show resolved Hide resolved
Documentation/spelling_wordlist.txt Show resolved Hide resolved
Documentation/helm-values.rst Outdated Show resolved Hide resolved
@bmcustodio
Copy link
Contributor Author

bmcustodio commented May 20, 2021

Another important point - What does the result look like on your setup? The generated table is very wide in my browser, and anything past the two or three first columns needs a lot of scrolling, making the table unreadable in practice. Have you tried alternative ways to render the values, or at least looking at table options to see if there's a way to wrap the content?

@qmonnet you are right, but unfortunately I don't know if there's a way to work around it — we have the same issue, I think, in some other places such as https://docs.cilium.io/en/v1.9/operations/metrics/#endpoint. I'll try to see what I can do.

@qmonnet
Copy link
Member

qmonnet commented May 20, 2021

Have you tried looking at the available options for the list-table directive?

@bmcustodio
Copy link
Contributor Author

Have you tried looking at the available options for the list-table directive?

I hadn't, thank you for the link 🙂 — unfortunately, I had a look and that doesn't seem to help much in making the table readable. I've pushed a CSS-based solution which, while not perfect, will hopefully work well 🙏🏼

Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/Makefile Outdated Show resolved Hide resolved
Documentation/spelling_wordlist.txt Show resolved Hide resolved
@nathanjsweet nathanjsweet merged commit de62fa3 into cilium:master Jun 7, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Needs backport from master in 1.8.11 Jun 10, 2021
@gandro
Copy link
Member

gandro commented Jun 10, 2021

I'm removing this from the 1.8 backport queue. If we really want this on 1.8, this needs a manual backport as the Helm setup is different there.

qmonnet added a commit to qmonnet/cilium that referenced this pull request Jun 17, 2021
PR cilium#16238 added a reference for the Helm values in the Charts to the
documentation. A number of these values are not common words from the
dictionary, and need to be added to the list of acceptable words in the
spelling list as we update the charts.

The GitHub action for documentation is supposed to help with the task,
catching omitted keywords. But it is only run when a number of
documentation-related files are run, and this does not currently include
the Charts! Let's fix in order to catch spelling mistake (or omitted
spelling list updates).

Fixes: cilium#16238
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
jibi pushed a commit that referenced this pull request Jun 22, 2021
PR #16238 added a reference for the Helm values in the Charts to the
documentation. A number of these values are not common words from the
dictionary, and need to be added to the list of acceptable words in the
spelling list as we update the charts.

The GitHub action for documentation is supposed to help with the task,
catching omitted keywords. But it is only run when a number of
documentation-related files are run, and this does not currently include
the Charts! Let's fix in order to catch spelling mistake (or omitted
spelling list updates).

Fixes: #16238
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@bmcustodio bmcustodio deleted the bmcustodio-docs-helm-reference branch June 24, 2021 08:01
tklauser pushed a commit to tklauser/cilium that referenced this pull request Jun 25, 2021
[ upstream commit 4e5272b ]

PR cilium#16238 added a reference for the Helm values in the Charts to the
documentation. A number of these values are not common words from the
dictionary, and need to be added to the list of acceptable words in the
spelling list as we update the charts.

The GitHub action for documentation is supposed to help with the task,
catching omitted keywords. But it is only run when a number of
documentation-related files are run, and this does not currently include
the Charts! Let's fix in order to catch spelling mistake (or omitted
spelling list updates).

Fixes: cilium#16238
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
tklauser pushed a commit to tklauser/cilium that referenced this pull request Jun 25, 2021
[ upstream commit 4e5272b ]

PR cilium#16238 added a reference for the Helm values in the Charts to the
documentation. A number of these values are not common words from the
dictionary, and need to be added to the list of acceptable words in the
spelling list as we update the charts.

The GitHub action for documentation is supposed to help with the task,
catching omitted keywords. But it is only run when a number of
documentation-related files are run, and this does not currently include
the Charts! Let's fix in order to catch spelling mistake (or omitted
spelling list updates).

Fixes: cilium#16238
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
joestringer pushed a commit that referenced this pull request Jun 28, 2021
[ upstream commit 4e5272b ]

PR #16238 added a reference for the Helm values in the Charts to the
documentation. A number of these values are not common words from the
dictionary, and need to be added to the list of acceptable words in the
spelling list as we update the charts.

The GitHub action for documentation is supposed to help with the task,
catching omitted keywords. But it is only run when a number of
documentation-related files are run, and this does not currently include
the Charts! Let's fix in order to catch spelling mistake (or omitted
spelling list updates).

Fixes: #16238
Signed-off-by: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.9 in 1.9.9 Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.9.9
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

9 participants