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.

Sign up for GitHub

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

Jump to bottom

Standardize helper comments and names (prefix all helper names with stackstorm-ha) #272

Merged
merged 12 commits into from
Jan 12, 2022
Merged

Standardize helper comments and names (prefix all helper names with stackstorm-ha) #272

merged 12 commits into from
Jan 12, 2022

Conversation

cognifloyd
Copy link
Member

This standardizes the template helpers to follow Helm's documented "Best Practices":

https://helm.sh/docs/chart_best_practices/templates/#comments-yaml-comments-vs-template-comments

Template comments should be used when documenting features of a template, such as explaining a defined template:

{{- /*
mychart.shortname provides a 6 char truncated version of the release name.
*/}}
{{ define "mychart.shortname" -}}
{{ .Release.Name | trunc 6 }}
{{- end -}}

https://helm.sh/docs/chart_best_practices/templates/#names-of-defined-templates

Defined templates (templates created inside a {{ define }} directive) are globally accessible. ...
For that reason, all defined template names should be namespaced.

Correct:

{{- define "nginx.fullname" }}
{{/* ... */}}
{{ end -}}

Incorrect:

{{- define "fullname" -}}
{{/* ... */}}
{{ end -}}
  • use helm-style comments in _helpers
  • prefix imageRepository helper with stackstorm-ha
  • prefix hyphenPrefix helper with stackstorm-ha
  • prefix st2-config-volume* helpers with stackstorm-ha
  • prefix init-containers-* helpers with stackstorm-ha
  • prefix pack-configs-volume* helpers with stackstorm-ha
  • prefix packs-volume* helpers with stackstorm-ha
  • prefix packs-initContainers helper with stackstorm-ha
  • prefix packs-pullSecrets helper with stackstorm-ha
  • add changelog entry

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Dec 2, 2021
cognifloyd
cognifloyd commented Dec 2, 2021
View reviewed changes
templates/_helpers.tpl Outdated Show resolved Hide resolved
templates/_helpers.tpl Outdated Show resolved Hide resolved
templates/_helpers.tpl Outdated Show resolved Hide resolved
@cognifloyd cognifloyd requested a review from arm4b December 2, 2021 04:43
@cognifloyd cognifloyd changed the title Misc helpers cleanups Standardize helper comments and names (prefix all helper names with stackstorm-ha) Dec 2, 2021
@cognifloyd cognifloyd added the Helm label Dec 2, 2021
@cognifloyd cognifloyd self-assigned this Dec 2, 2021
ericreeves
ericreeves approved these changes Dec 2, 2021
View reviewed changes
Copy link
Contributor

@ericreeves ericreeves left a comment

Choose a reason for hiding this comment

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

Best practices are a good thing. This helps bring clarity and specificity to chart values and defines.

arm4b
arm4b approved these changes Jan 12, 2022
View reviewed changes
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@cognifloyd cognifloyd merged commit 06103d0 into StackStorm:master Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericreeves ericreeves ericreeves approved these changes

@arm4b arm4b arm4b approved these changes

Assignees

@cognifloyd cognifloyd

Labels
Helm size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cognifloyd @arm4b @ericreeves