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

Making all annotations multi-line strings #227

Merged
merged 1 commit into from
Mar 20, 2020

Conversation

tvoran
Copy link
Member

@tvoran tvoran commented Mar 14, 2020

Annotations for various objects were either multi-line strings or yaml
maps strings, so this is making them all multi-line strings for
consistency. Also updated the doc comment for namespaceSelector, since
it's being read as a yaml map (toYaml).

Relates to #117

Annotations for various objects were either multi-line strings or yaml
maps strings, so this is making them all multi-line strings for
consistency. Also updated the doc comment for namespaceSelector, since
it's being read as a yaml map (toYaml).
@tvoran tvoran added chart Area: helm chart enhancement New feature or request labels Mar 14, 2020
@jasonodonnell jasonodonnell self-requested a review March 19, 2020 00:51
@tvoran tvoran requested a review from spangenberg March 19, 2020 05:15
Copy link

@spangenberg spangenberg left a comment

Choose a reason for hiding this comment

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

LGTM

@tvoran tvoran merged commit 2a37c57 into master Mar 20, 2020
@tvoran tvoran deleted the values-annotations-consistency branch March 20, 2020 15:37
@nesl247
Copy link

nesl247 commented Apr 9, 2020

Why would you do a multi-line string instead of a yaml hash for this? This is the "standard" used in pretty much any helm chart I've seen.

@tvoran
Copy link
Member Author

tvoran commented Apr 9, 2020

I think the main reason is that a multi-line string can be run through tpl so that chart values will be interpolated, and for custom annotations at least that seems like a useful thing. The other reason is the sister chart to this one (consul-helm) follows a similar convention.

In any case, this PR was just about making annotations consistent within the chart.

@nesl247
Copy link

nesl247 commented Apr 9, 2020

I guess that makes sense for people using helm directly. I'm using pulumi which is like terraform, so I let pulumi handle that part.

Either way, I'll discuss this more in #55.

kri5 added a commit to kri5/vault-helm that referenced this pull request Apr 15, 2020
It has been decided in hashicorp#227 to move annotations to multi-line
string for consistency. Thoses definitions were not following thoses
rules. This commit fixes it.
radudd pushed a commit to radudd/vault-helm that referenced this pull request Jun 5, 2020
Annotations for various objects were either multi-line strings or yaml
maps strings, so this is making them all multi-line strings for
consistency. Also updated the doc comment for namespaceSelector, since
it's being read as a yaml map (toYaml).
hadielaham88 pushed a commit to SolaceDev/vault-helm that referenced this pull request May 19, 2021
Annotations for various objects were either multi-line strings or yaml
maps strings, so this is making them all multi-line strings for
consistency. Also updated the doc comment for namespaceSelector, since
it's being read as a yaml map (toYaml).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart Area: helm chart enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants