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

README and issue template updates #3818

Merged
merged 4 commits into from
Nov 16, 2017

Conversation

chrislovecnm
Copy link
Contributor

@chrislovecnm chrislovecnm commented Nov 10, 2017

Addresses #3813

  • add information about new features
  • removing maintainers section
  • adding GCE documentation to README
  • reformating the issues template

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 10, 2017
@chrislovecnm
Copy link
Contributor Author

/assign @geojaz @andrewsykim

@chrislovecnm chrislovecnm force-pushed the readme-updates branch 2 times, most recently from 62e688f to 3a8e801 Compare November 10, 2017 06:09
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 10, 2017
Copy link
Member

@geojaz geojaz left a comment

Choose a reason for hiding this comment

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

Nothing major here, just a few edits. thanks @chrislovecnm.


6. What you expected to happen:
1. Can you provide cluster manifest `kops get --name my.example.com -oyaml`?
Copy link
Member

Choose a reason for hiding this comment

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

How about:
Please provide your cluster manifest kops get --name my.example.com -oyaml if possible.


7. How can we to reproduce it (as minimally and precisely as possible):
1. What you expected to happen?
Copy link
Member

Choose a reason for hiding this comment

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

Your numbers got reset here. Silly markdown!

What did you expect to happen when you ran this? What would you like this command to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you just do 1. 1. 1. you do not have to worry about changing them ever. The render with the correct numbers ;)


8. Anything else do we need to know:
1. How can we to reproduce it (as minimally and precisely as possible)?
Copy link
Member

Choose a reason for hiding this comment

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

-to and lose the parens

What is the simplest way to reproduce this result?


-------------FEATURE REQUEST --------------------
1. Can you run the `kops` command with the `-v 10` flag and provide logs?
Copy link
Member

Choose a reason for hiding this comment

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

Please run the command with most verbose logging by adding the -v 10 flag and paste the logs into this report.

README.md Outdated
## Kubernetes Release Compatibility

`kops` is intended to be backward compatible. It is always recommended to use the
latest version of kop with whatever version of Kuberentes you are using. This
Copy link
Member

Choose a reason for hiding this comment

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

s/kop/kops, s/Kuberentes/Kubernetes


2. What kops version are you running? use `kops version`
1. What kops version are you running? Use `kops version`?
Copy link
Member

Choose a reason for hiding this comment

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

I would change the "Use kops version?" to something like:

(run kops version)

Copy link
Member

Choose a reason for hiding this comment

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

Concur. This isn't a question.


3. What Kubernetes version are you running? use `kubectl version`
1. What Kubernetes version are you running? Use `kubectl version`?
Copy link
Member

Choose a reason for hiding this comment

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

☝️


5. What commands did you execute (Please provide cluster manifest `kops get --name my.example.com`, if available) and what happened after commands executed?
1. What commands did you execute, and what happened after the commands executed?
Copy link
Member

Choose a reason for hiding this comment

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

What commands did you run and what happened?


2. What kops version are you running? use `kops version`
1. What kops version are you running? Use `kops version`?
Copy link
Member

Choose a reason for hiding this comment

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

Concur. This isn't a question.

README.md Outdated
`kops` is intended to be backward compatible. It is always recommended to use the
latest version of kop with whatever version of Kuberentes you are using. This
project does not follow the Kubernetes release schedule but typically releases
after Kubernetes does. `kops` supports the matching Kubernetes minor release.
Copy link
Member

Choose a reason for hiding this comment

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

I would split out the "This project does not follow the kubernetes release schedule..." sentence into is own paragraph, after this paragraph.

I would also say why we don't follow the kubernetes release schedule. Something like:

"kops aims to provide a reliable installation experience for kubernetes, and typically releases about a month after the corresponding Kubernetes release. This time allows for release issues in kubernetes, and ensures that we can support the latest features. Kops will release alpha and beta pre-releases before then for people that are eager to try the latest kubernetes release and have environments where they can tolerate the quirks of new releases - and please do report any issues encountered."

@chrislovecnm
Copy link
Contributor Author

@geojaz @justinsb PTAL

README.md Outdated
the latest version of kops with whatever version of Kubernetes you are using.
`kops` supports the matching Kubernetes minor release. For instance kops,
1.8.x does not support Kubernetes 1.9.x, but kops 1.9.x supports Kubernetes
1.9.x. Moreover, kops 1.8.x attempts to be backward compatible and support

Choose a reason for hiding this comment

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

This should say "but kops 1.9.x would support kubernetes 1.8.x" right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

README.md Outdated
This project does not follow the Kubernetes release schedule. `kops` aims to
provide a reliable installation experience for kubernetes, and typically
releases about a month after the corresponding Kubernetes release. This time
allows for release issues in Kubernetes and ensures that we can support the

Choose a reason for hiding this comment

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

"allows for release issues to be resolved"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is passive voice, let me see if I can make it active voice.

Copy link

@blakebarnett blakebarnett left a comment

Choose a reason for hiding this comment

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

👍

README.md Outdated

To replicate the above demo, check out our [tutorial](/docs/aws.md) for
launching a Kubernetes cluster hosted on AWS.

To install a Kuberentes cluster on GCE please follow this [guide](/docs/tutorial/gce.md).
Copy link
Member

Choose a reason for hiding this comment

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

kuberentes :)

@chrislovecnm
Copy link
Contributor Author

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 10, 2017
Copy link
Member

@geojaz geojaz left a comment

Choose a reason for hiding this comment

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

Good stuff Chris. The others had excellent feedback as well. once you get rid of the kuberentes, I can approve!

@chrislovecnm
Copy link
Contributor Author

@geojaz I am re-writing the versioning section again. Will ping you

@chrislovecnm
Copy link
Contributor Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 10, 2017
@chrislovecnm
Copy link
Contributor Author

@blakebarnett @geojaz PTAL

I added more details to the versioning section. It is complicated, so I tried to provide information and examples. Also, I deliberately repeated certain statements.

README.md Outdated
Kubernetes release. kops does not stop a user from installing mismatching
versions of K8s, but Kubernetes releases always require kops to install specific
versions of components like docker, that tested against the particular
Kuberenetes version.
Copy link
Member

Choose a reason for hiding this comment

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

they're breeding bro!

Copy link
Member

Choose a reason for hiding this comment

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

This is helpful but I almost think we need a diagram or a matrix in addition. Does not need to be a part of this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Never said I could spell. I am going to add a table matix here

README.md Outdated
versions of components like docker, that tested against the particular
Kuberenetes version.

Use the latest version of kops for all releases of Kuberentes, with the caveat
Copy link
Member

Choose a reason for hiding this comment

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

👉

README.md Outdated
### Kubernetes Version Support

kops is intended to be backward compatible. It is always recommended to use the
latest version of kops with whatever version of Kubernetes you are using. One
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little bit confused about what you're saying here. Are you saying that regardless of the k8s version, use the latest released kops?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct

Copy link
Member

@geojaz geojaz left a comment

Choose a reason for hiding this comment

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

not picking on you. more little things :)

README.md Outdated
Kuberenetes version.

Use the latest version of kops for all releases of Kuberentes, with the caveat
that higher versions of Kuberentes are not _officially_ supported by kops.
Copy link
Member

Choose a reason for hiding this comment

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

👉 here too

@chrislovecnm
Copy link
Contributor Author

@geojaz / @blakebarnett PTAL

@blakebarnett
Copy link

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2017
@chrislovecnm
Copy link
Contributor Author

chrislovecnm commented Nov 16, 2017

/approve

1 similar comment
@chrislovecnm
Copy link
Contributor Author

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2017
@chrislovecnm
Copy link
Contributor Author

/approve cancel

@k8s-ci-robot k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2017
@chrislovecnm
Copy link
Contributor Author

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2017
@blakebarnett
Copy link

hah, I only searched for kuberentes

@k8s-github-robot
Copy link

/lgtm cancel //PR changed after LGTM, removing LGTM. @andrewsykim @blakebarnett @chrislovecnm @geojaz

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2017
@blakebarnett
Copy link

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2017
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: blakebarnett, chrislovecnm

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot
Copy link

Automatic merge from submit-queue.

@k8s-github-robot k8s-github-robot merged commit aa7ee34 into kubernetes:master Nov 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants