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

Update NOTES.txt, including removing "alpha" designation #2165

Merged
merged 5 commits into from
Apr 28, 2021

Conversation

manics
Copy link
Member

@manics manics commented Apr 24, 2021

Also includes direct commands for extracting IPs/NodePort (not sure if this is too complicated? though it makes it easier for people who don't immediately understand how to parse kubectl get svc proxy-public)

TODO:

  • ingress?

@manics manics changed the title WIP: Update NOTES.txt, including removing "alpha" designation Update NOTES.txt, including removing "alpha" designation Apr 27, 2021
@manics manics marked this pull request as ready for review April 27, 2021 20:08
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

This LGTM, small whitespace detail I think could be addressed.

Comment on lines 51 to 53
3. If you find a bug please report it at https://github.com/jupyterhub/zero-to-jupyterhub-k8s/issues{{ println }}

{{- if hasKey .Values.singleuser.cloudMetadata "enabled" }}{{ println }}
{{- if hasKey .Values.singleuser.cloudMetadata "enabled" }}
Copy link
Member

Choose a reason for hiding this comment

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

Assuming someone would copy paste the deprecation logic, they would need {{ println }} in it at least assuming two deprecation messages would trigger - in order to avoid no new blank line between them.

Copy link
Member

Choose a reason for hiding this comment

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

Whenever there is a fail it is not relevant with later messages as it would stop the rendering, but if it is a warning - multiple warnings can trigger.

Below there is a deprecation warning like this that includes println still while this doesn't any more. I suggest we consistently use println within each.

@consideRatio consideRatio merged commit 0119333 into jupyterhub:master Apr 28, 2021
@consideRatio
Copy link
Member

@manics thank you so much for maintaining, well, everything !! ❤️

consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request Apr 28, 2021
@manics manics deleted the update-notes branch April 28, 2021 21:13
@manics manics mentioned this pull request May 5, 2021
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants