{# comments #} replaced with # comments

[consideRatio]

A simple syntax error with a simple fix - I think! In this PR I simply replaced {# these types of comments #} with # these types of comments.

Perhaps {# this comment syntax #} is valid, but if so it seem to have caused some indentation errors. I got an error on my helm upgrade ... command that appears to be commonly associated with indentation issues.

Error:
  UPGRADE FAILED:
    YAML parse error on z2jh-extended/charts/jupyterhub/templates/hub/deployment.yaml:
      error converting YAML to JSON: yaml: line 111: could not find expected ':'

Ping #597 @mattjbray @yuvipanda

[yuvipanda]

The one difference between these two is that the plain {# #} comments will never be emitted into the rendered YAML, while the # comments will be. This makes a lot of difference in some places - primarily in configmaps / other places where literal values are interpolated. We should probably make a style guide for this repo laying things like this out...

I'm not opposed to this :) Does this fix the issue, @consideRatio?

[consideRatio]

Ah yes it did for me, but okay so then we might need to add something like {#- this? -#} perhaps in order to have it function without causing indentation errors?

I'll try!

[consideRatio]

BTW; googling for this was tough, what are good keywords? Got an URL or similar to read about these kinds of comments?

[consideRatio]

I tried the following combinations, only normal comment option avoided the could not find expected ':' error.

# comment 1
{# comment 2 #}
{#- comment 3 -#}
{#- comment 4 #}
{# comment 5 -#}

[yuvipanda]

Yeah, it's pretty difficult. I knew this from reading https://golang.org/pkg/text/template/ (and other resources on searching for 'go templates', which is what helm templates are). Perhaps things to add to our contributing.md?

…

On Sat, Mar 31, 2018 at 8:30 PM, Erik Sundell ***@***.***> wrote: BTW; googling for this was tough, what are good keywords? Got an URL or similar to read about these kinds of comments? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#618 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB23o0arsfFCcHBli1IrZxezE2XGzwVks5tkEnogaJpZM4TCshy> .

-- Yuvi Panda T http://yuvi.in/blog

[yuvipanda]

Oh wow, the syntax is actually `{{/* */}}`. Maybe that's why you are getting indent errors? {# #} is valid Jinja2 syntax I think, but *not* valid go template syntax.

…

On Sat, Mar 31, 2018 at 8:23 PM, Erik Sundell ***@***.***> wrote: A simple syntax error with a simple fix - I think! In this PR I simply replaced {# these types of comments #} with # these types of comments. Perhaps {# this comment syntax #} is valid, but if so it seem to have caused some indentation errors. I got an error on my helm upgrade ... command that appears to be commonly associated with indentation issues. Error: UPGRADE FAILED: YAML parse error on z2jh-extended/charts/jupyterhub/templates/hub/deployment.yaml: error converting YAML to JSON: yaml: line 111: could not find expected ':' Ping #597 <#597> @mattjbray <https://github.com/mattjbray> @yuvipanda <https://github.com/yuvipanda> ------------------------------ You can view, comment on, or merge this pull request online at: #618 Commit Summary - Comments {# #} replaced with # File Changes - *M* jupyterhub/templates/hub/deployment.yaml <https://github.com/jupyterhub/zero-to-jupyterhub-k8s/pull/618/files#diff-0> (4) Patch Links: - https://github.com/jupyterhub/zero-to-jupyterhub-k8s/pull/618.patch - https://github.com/jupyterhub/zero-to-jupyterhub-k8s/pull/618.diff — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#618>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAB23rGcwpJH7oheJel9h4KUzi1S0X2xks5tkEg1gaJpZM4TCshy> .

-- Yuvi Panda T http://yuvi.in/blog

[consideRatio]

Yepp! I saw it actually was {{\* *\}} and by doing that it worked!

[yuvipanda]

LGTM, Thanks for the PR!

[consideRatio]

@yuvipanda ❤️