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

Move kubectl overview to be section index #29847

Merged

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Sep 28, 2021

Also:

  • use glossary definition in page introduction
  • tidy broken link in What's Next section
  • redirect old URL to new URL

/sig cli
/language en
/kind cleanup

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. sig/cli Categorizes an issue or PR as relevant to SIG CLI. language/en Issues or PRs related to English language kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 28, 2021
@k8s-ci-robot k8s-ci-robot added the sig/docs Categorizes an issue or PR as relevant to SIG Docs. label Sep 28, 2021
@netlify
Copy link

netlify bot commented Sep 28, 2021

✔️ Deploy Preview for kubernetes-io-main-staging ready!

🔨 Explore the source changes: 77a598c

🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/62209391ca15f200086a5a23

😎 Browse the preview: https://deploy-preview-29847--kubernetes-io-main-staging.netlify.app

@sftim sftim marked this pull request as ready for review October 2, 2021 16:41
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 2, 2021
@sftim
Copy link
Contributor Author

sftim commented Oct 2, 2021

An enabling change for #25943

@sftim sftim force-pushed the 20210928_migrate_kubectl_overview branch from 168b095 to dafeaa1 Compare October 7, 2021 09:19
@@ -4,16 +4,19 @@ id: kubectl
date: 2018-04-12
full_link: /docs/user-guide/kubectl-overview/
short_description: >
A command line tool for communicating with a Kubernetes API server.
A command line tool for communicating with a Kubernetes cluster.
Copy link
Contributor

Choose a reason for hiding this comment

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

A command line tool for communicating with a Kubernetes cluster API server.

Copy link
Contributor

Choose a reason for hiding this comment

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

for me both is right depending on the target group

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the tooltip text, shown when you hover. You can always click on the tooltip to learn more. I think this text is right for a tooltip.

Copy link
Contributor

Choose a reason for hiding this comment

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

Calling out, "Kubernetes API server" seems more specific than "cluster". Is it more important to identify the Kubernetes API server?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's why I prefer “cluster”: if we say “API server” in the tooltip, I'm worried that some newcomers will feel that they need to learn what a Kubernetes API server is—not an easy thing to grasp—and that some proportion of those will just write off Kubernetes as too hard to learn.

The full description is welcome to make the details clear.

## {{% heading "whatsnext" %}}

* Read the `kubectl` [command reference](/docs/reference/kubectl/kubectl/).

Copy link
Contributor

Choose a reason for hiding this comment

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

Find some further useful command examples on the kubectl cheat sheet

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe useful to add

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I usually prefer to keep migration PRs separate from PRs that add new content.

@sftim
Copy link
Contributor Author

sftim commented Oct 26, 2021

@mkorbi have I convinced you that we can accept these changes as-is? Happy to discuss further.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 25, 2021
@sftim sftim force-pushed the 20210928_migrate_kubectl_overview branch from dafeaa1 to 9690fda Compare December 5, 2021 23:02
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2021
@sftim sftim force-pushed the 20210928_migrate_kubectl_overview branch from cc29133 to fee70f0 Compare December 5, 2021 23:10
@jimangel
Copy link
Member

jimangel commented Dec 6, 2021

/lgtm

I wonder if this should land after the release and if we need to boost visibility to localization (to me it feels like a small big change vs. a refactor)?

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

LGTM label has been added.

Git tree hash: 7c71f4302318adc08eff1480816accd0c088dbee

@sftim
Copy link
Contributor Author

sftim commented Dec 6, 2021

Happy to let this land after the v1.23 release and yes, we should consider applying the refactor label.
/label refactor

@k8s-ci-robot k8s-ci-robot added the refactor Indicates a PR with large refactoring changes e.g. removes files or moves content label Dec 6, 2021
@sftim sftim force-pushed the 20210928_migrate_kubectl_overview branch from fee70f0 to a6ebe69 Compare December 8, 2021 02:01
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 8, 2021
@sftim
Copy link
Contributor Author

sftim commented Jan 5, 2022

@jimangel the only changes since your previous LGTM were a (quite big) rebase with no manual rework

## {{% heading "whatsnext" %}}

* Read the `kubectl` [command reference](/docs/reference/kubectl/kubectl/).
* Read the `kubectl` [command line arguments](/docs/reference/kubectl/kubectl/) reference.
Copy link
Contributor

Choose a reason for hiding this comment

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

👋 @sftim, Can you check the links for the first two items (links are the same)?
The page could be configured to ignore the list of section links at the bottom of the page, or do you think the links add value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed?

@@ -1,5 +1,551 @@
---
title: "kubectl"
title: Command line tool (kubectl)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Should the title contain Kubernetes?
Kubernetes command-line tool (kubectl)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kbhawkey let's save that for a different PR; that would be a change beyond the page move that this PR's title covers.

sftim added 3 commits March 3, 2022 10:03
Also:
- use glossary definition in page introduction
- tidy broken link in What's Next section
- update links to refer to moved page
Also, reorder the section overall.
@sftim sftim force-pushed the 20210928_migrate_kubectl_overview branch from f8c79a0 to 77a598c Compare March 3, 2022 10:08
@kbhawkey
Copy link
Contributor

kbhawkey commented Mar 5, 2022

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kbhawkey

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 5, 2022
@kbhawkey
Copy link
Contributor

kbhawkey commented Mar 5, 2022

/lgtm

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

LGTM label has been added.

Git tree hash: f2f96258cb8f54c401590ed95201140accd6faa3

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. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. refactor Indicates a PR with large refactoring changes e.g. removes files or moves content sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants