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

create simplified kubectl ref #132

Closed

Conversation

kbhawkey
Copy link
Contributor

@kbhawkey kbhawkey commented Jan 27, 2020

  • Built simple, single page kubectl command reference.
  • [TODO] Description, Usage, Examples text. Really any descriptive text should be converted or cleaned up. I found that the text has a number of markup and special chars such as: important note (!!!), embedded links, embedded lists. Newlines could be preserved and whitespace. Possibly convert Markdown snippets to HTML (goldmark), then run through golang template.Html.
  • [TODO] Overall page styling needs more work. Determine colors, highlight text, etc. Verify static assets and clean up.
  • [TODO] Remove static-includes directory and files and update setup script.
  • Flags are presorted (alphabetical); does this need additional verification.
  • [TODO] Add table captions (Aria).
  • Simplified Makefile
  • See issue migrate kubectl reference #118.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 27, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kbhawkey
To complete the pull request process, please assign bradamant3
You can assign the PR to them by writing /assign @bradamant3 in a comment when ready.

The full list of commands accepted by this bot can be found 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 size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 27, 2020
@kbhawkey
Copy link
Contributor Author

/cc @tengqm

@kbhawkey kbhawkey changed the title create siimplified kubectl ref [wip] create simplified kubectl ref Jan 27, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 27, 2020
@kbhawkey
Copy link
Contributor Author

image

for _, c := range toc.Categories {

// Write each of the commands in this category
for _, cm := range c.Commands {
Copy link
Contributor

Choose a reason for hiding this comment

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

any consideration on the ordering of values in the map?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it is okay at this stage ... eventually we need an ordered list of the commands and options so that the output can be easily compared to each other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thinking of sorting the commands based on the order/grouping of commands in toc.yaml. I updated toc.yaml to match the output of kubectl help.
Ideally, the group name/message would be provided by the spec (or kubectl help).

Copy link
Contributor Author

@kbhawkey kbhawkey Jan 29, 2020

Choose a reason for hiding this comment

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

The commands are ordered per category and the sub-commands are provided through the spec. The categories are sorted by the groupings in the toc.yaml file.
I guess the order of the sub-commands and options could change from release to release. I think sorting the flags/options would be beneficial.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

"<tbody>"

for _, option := range cmd.Options {
buf += "<tr><td>" + option.Name + "</td><td>" + option.Shorthand + "</td><td>" +
Copy link
Contributor

Choose a reason for hiding this comment

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

option.Name may need some tags like <code>.

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 kbhawkey force-pushed the kb-kubectlref-fixes branch from 2c93d8b to 11b9ed0 Compare January 29, 2020 03:14
add categories

more updates

updates
@kbhawkey kbhawkey force-pushed the kb-kubectlref-fixes branch from 11b9ed0 to ad8bee2 Compare January 30, 2020 14:46
@tengqm
Copy link
Contributor

tengqm commented Mar 9, 2020

@kbhawkey Is this still a WIP? Can we kick it in and iterate over it?

@kbhawkey
Copy link
Contributor Author

kbhawkey commented Mar 9, 2020

@kbhawkey Is this still a WIP? Can we kick it in and iterate over it?

@tengqm , This is sort of a WIP. I started to work on cleaning up the Examples section, but have not gotten back to it yet. I was not sure if there was any interest in this PR.
I am definitely in favor a new process and UI for this reference.
If the examples were fixed, then I would consider this PR less of a WIP. There are a lot of style tweaks that could be made, but I am happy with the basic way the docs generate.
The description text, examples text, and description text of flags needs to be cleaned up (Markdown, odd formatting, etc.). I will reopen my k8s/website PR which shows the full reference built using the code from this PR. Have a look again and let me know.
Do you have other ideas?

@tengqm
Copy link
Contributor

tengqm commented Mar 10, 2020

@kbhawkey You are right we need to tune the look&feel of examples. There are other things to tune as well, such as the style for the left navigator. I was hoping that we can make it auto-hiding.
As a useful option, I'd vote for getting this in, even if it is not favored by the website team.

@kbhawkey
Copy link
Contributor Author

Since it has been some time, I will look at the state of the changes (and changes I made but did not merge in yet).
Yes, I agree to iterate and move these changes along.

@@ -0,0 +1,61 @@
categories:
- name: GETTING STARTED

Choose a reason for hiding this comment

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

Around content organization, perhaps a grouping could be APPLICATION MANAGEMENT?

Going even further, I'd prefer to have groupings for IMPERATIVE APPLICATION MANAGEMENT and DECLARATIVE APPLICATION MANAGEMENT to mirror the corresponding doc sections under Manage Kubernetes Objects:

Declarative Management of Kubernetes Objects Using Configuration Files
Declarative Management of Kubernetes Objects Using Kustomize
Managing Kubernetes Objects Using Imperative Commands
Imperative Management of Kubernetes Objects Using Configuration Files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@julianvmodesto, Should the group names be restricted to a specific length? The names are part of the left navigation.

Is it possible for kubectl to provide the groupings or categories. The toc could be dynamically generated instead of manually updated whenever the commands change.
I think there is an issue/PR in spf13/cobra requesting something like this (spf13/cobra#836).

@kbhawkey kbhawkey changed the title [wip] create simplified kubectl ref create simplified kubectl ref Mar 10, 2020
@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 Mar 10, 2020
@kbhawkey
Copy link
Contributor Author

Before adding this in, is there time (v1.18 release) to get the reference to a point where it is no longer a draft and is an acceptable replacement for the current kubectl command reference (granted I think the current ref is difficult to use and looks outdated).

@kbhawkey
Copy link
Contributor Author

Placing a hold on this again.
/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 Mar 21, 2020
@kbhawkey
Copy link
Contributor Author

kbhawkey commented May 1, 2020

/close

@k8s-ci-robot
Copy link
Contributor

@kbhawkey: Closed this PR.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot
Copy link
Contributor

@kbhawkey: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. 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.

4 participants