Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

docs(managing-app-configuration): update "kubernetes-probes" link #773

Merged
merged 1 commit into from
Mar 16, 2017

Conversation

tcg
Copy link
Contributor

@tcg tcg commented Mar 16, 2017

The previous link, https://kubernetes.io/docs/user-guide/pod-states/#container-probes, displays a message about Kubernetes docs being moved, and includes a link to the new location for the particular topic. (And no longer includes the content/documentation).

I've updated the link to the new location, and proper heading anchor, in the Kubernetes Pod Lifecycle documentation: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#container-probes

@deis-bot
Copy link

@bacongobbler, @jeroenvisser101 and @mboersma are potential reviewers of this pull request based on my analysis of git blame information. Thanks @tcg!

@mboersma
Copy link
Member

@tcg would you mind amending your commit message to match our contribution style guide? Otherwise it won't get included in the CHANGELOG for a new release. Something like:

docs(managing-app-configuration): update "container probes" link

@tcg tcg changed the title Update "container probes" link in docs docs(managing-app-configuration): update "kubernetes-probes" link Mar 16, 2017
The previous link,
https://kubernetes.io/docs/user-guide/pod-states/#container-probes,
displays a message about Kubernetes docs being moved, and includes
a link to the new location for the particular topic.
I've updated the link to the new location, and proper heading anchor,
in the Kubernetes Pod Lifecycle documentation:
https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#container-probes
@tcg tcg force-pushed the tcg/managing-app-conf-link branch from 8c0ae60 to 3df978c Compare March 16, 2017 22:17
@tcg
Copy link
Contributor Author

tcg commented Mar 16, 2017

@mboersma Thanks!

For the record, I did check the readme, which doesn't mention the standard. Then I looked in the CONTRIBUTING.md, which contains a link. That link goes to the Overview page, which does contain a heading about Documentation, specifically simplifying it as "Simply fork the project, update docs and send us a pull request."

While I think it's cool, handy, and useful to use that standard, I did not consider delving further to find the spec, for what I assumed was a 5 minute change. =) You didn't say it was necessary, so in that case I have no problem with the process. If I'm wrong, however, it would be useful to point to the standard "sooner".

Thanks for your help!

@bacongobbler
Copy link
Member

I can see that. If you're not concerned about getting your name into the release notes for a flyby PR then that's fine. :)

@mboersma
Copy link
Member

mboersma commented Mar 16, 2017

it would be useful to point to the standard "sooner".

The first time you submit a PR to any Workflow-related project except this one, our Jenkins CI system will respond with a "please ensure you follow the coding standards" message that has the same link I posted. (Once you're "whitelisted," it stops reminding you.) But this repo deis/workflow isn't actually hooked up to Jenkins, just Travis CI, so it doesn't get that benefit. 😞

Anyway, thanks for fixing the commit message up and sorry for not making it more obvious in this case. We should add a commit template here that reminds users about such expectations when they open a PR.

@mboersma mboersma merged commit 2269596 into deis:master Mar 16, 2017
@tcg
Copy link
Contributor Author

tcg commented Mar 16, 2017

Thanks, all.

I'm not concerned with credit, but others may be. =)

Sorry to be so verbose -- I hope I came across and being helpful and more confused than perturbed. Just wanted to suggest an improvement, if it was something that I should have been expected to have seen.

Regardless, @mboersma, your tone was kind, and I appreciate how you made the suggestion to someone new to the Deis project. Good job! 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants