Skip to content

[WIP] Testing restyled kubectl ref docs#82

Closed
kbhawkey wants to merge 3 commits intokubernetes-sigs:masterfrom
kbhawkey:kubectl-gen-testing
Closed

[WIP] Testing restyled kubectl ref docs#82
kbhawkey wants to merge 3 commits intokubernetes-sigs:masterfrom
kbhawkey:kubectl-gen-testing

Conversation

@kbhawkey
Copy link
Copy Markdown
Contributor

@kbhawkey kbhawkey commented Aug 8, 2019

No description provided.

@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 Aug 8, 2019
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 8, 2019
@xiangpengzhao
Copy link
Copy Markdown

/assign @tengqm

@jimangel
Copy link
Copy Markdown
Member

Is there an example output for this anywhere @kbhawkey?

sudo rm -rf $(shell pwd)/gen-kubectldocs/generators/manifest.json

cli: cleancli
# go mod init github.com/generators
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if the output is something like what we now get from line 43.

Copy link
Copy Markdown
Contributor Author

@kbhawkey kbhawkey Aug 16, 2019

Choose a reason for hiding this comment

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

Yes, but with different front matter, heading changes, and integrated sub-commands. I updated the go template.

@kbhawkey
Copy link
Copy Markdown
Contributor Author

kbhawkey commented Aug 16, 2019

Is there an example output for this anywhere @kbhawkey?

Hi @jimangel, @tengqm . Yes, I created a PR in k8s/website with the generated output.
See kubernetes/website#15742.
A couple of ideas I have been thinking about:

  • The code needs further clean up. There is more code that can be removed/updated. I made some initial edits.
  • Could the reference docs use go modules?
  • Could the gen script for kubectl reside with the kubectl code, but use the hugo k8s/website reference templating to generate docs? Or leave the gen script here. Reach out to sig/cli.
  • Motivation: Reduce the amount of assets that are required for the reference docs (kubectl reference, specifically) and reuse the templating and assets from k8s/website docs html generation (hugo). Create a reference doc for the kubectl commands that looks like it fits in with the rest of the k8s/website. Make it easy to generate reference docs and easy to update the command text if needed.
  • The make cli command in this Makefile generates individual markdown files per main Commands; the sub-commands are included with the main Commands. Right now, there is a top nav list on the page to link up the sub commands (caused by making H2, H3 heading levels). This could change if another content page template is adopted for the reference pages. I reused the reference template that I created some time ago -- could create a separate one for kubectl. I just made a few of the sections optional instead of required. There are a number of other open source reference/api docs that use a right hand side nav list.
  • The flags table could use a different set of styling than the other k8s/website tables -- there are a lot of possibilities to improve the readability and navigation of the pages. The font is too small.

@kbhawkey
Copy link
Copy Markdown
Contributor Author

Hi, @pwittrock. This PR represents some initial ideas about aligning the generation of the kubectl reference doc with the kubenetes.io publishing toolchain, hugo. I think the kubectl reference doc is useful. Would you have time to provide feedback? Thank you!
Page preview:
https://deploy-preview-15742--kubernetes-io-master-staging.netlify.com/docs/reference/kubectl/kubectl-reference/alpha/

@tengqm
Copy link
Copy Markdown
Contributor

tengqm commented Aug 22, 2019

@kbhawkey If you feel good about the status of the PR, we can kick it in. Approval pending from my side because there is a [wip] in the PR title. :)

@kbhawkey
Copy link
Copy Markdown
Contributor Author

kbhawkey commented Aug 22, 2019

@kbhawkey If you feel good about the status of the PR, we can kick it in. Approval pending from my side because there is a [wip] in the PR title. :)

hello @tengqm . I am just getting back to this. I'd like another day or so as I am investigating an aside toc for the pages.

[NEW] @tengqm
I just went through the generation process again. A couple of things came up:

  • With these changes, how will the former versions of the reference be maintained or regenerated? Create a new branch?
  • There is some more to cleanup and there will be more changes once the styling is updated on the k8s/website hugo side of things.
  • I tried to create a go module file, but some of the dependencies were not resolved.
  • The toc.yaml file works, but I did not clean this up yet.
  • Need to add a table caption to the flags table. Possibly add aria-label too.
  • Let me know what you think. Thanks!

added version line to template
<a href="https://github.com/kubernetes/kubernetes">Kubectl Reference Docs version 1.15 &#xa9;Copyright 2019 The Kubernetes Authors</a>
{{"<a href=\"https://github.com/kubernetes/kubernetes\">Kubectl Reference Docs "}}
{{"{{< param \"fullversion\" >}} "}} {{"&#xa9;Copyright 2019 The Kubernetes Authors</a>"}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@nikhita
Copy link
Copy Markdown
Member

nikhita commented Sep 26, 2019

👋 just wanted to check if there are any updates here since this is blocking the migration from incubator to k-sigs :)

cc @mrbobbytables

@mrbobbytables
Copy link
Copy Markdown
Member

pinging @jimangel to loop back in as well 👍

@kbhawkey
Copy link
Copy Markdown
Contributor Author

👋 just wanted to check if there are any updates here since this is blocking the migration from incubator to k-sigs :)

cc @mrbobbytables

hello @nikhita . I am still (temporarily took a break) working on this. Why is the PR blocking migration? I can close and resume once the repository is moved. Where can I find information about the repo move?
Thanks!

@jimangel
Copy link
Copy Markdown
Member

@nikhita @mrbobbytables considering the 2 open PRs are major changes with no short term plan on merging, I think we can copy the repo ASAP (let them co-exist for a finite amount of time - 2 weeks? sooner?) and set a date to close this repo.

@kbhawkey info about the move: kubernetes/org#1158

/cc @tengqm

@k8s-ci-robot k8s-ci-robot requested a review from tengqm September 26, 2019 16:27
@kbhawkey
Copy link
Copy Markdown
Contributor Author

[ I think we can copy the repo ASAP (let them co-exist for a finite amount of time - 2 weeks? sooner?) and set a date to close this repo.]
@jimangel , that is fine -- I still have the branch. I guess this will not wipe out my fork of kubernetes-incubator/reference-docs.

@tengqm
Copy link
Copy Markdown
Contributor

tengqm commented Sep 27, 2019

@kbhawkey Maybe we can just close this PR and reopen it after the migration?

@mrbobbytables
Copy link
Copy Markdown
Member

@kbhawkey Maybe we can just close this PR and reopen it after the migration?

The PR should actually move over as long as the repo is migrated. You'll need to change your git config locally and set upstream to be k-sigs based instead of incubator though.

@kbhawkey
Copy link
Copy Markdown
Contributor Author

Closing -- moving to a new location!
/close

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@kbhawkey: Closed this PR.

Details

In response to this:

Closing -- moving to a new location!
/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.

@kbhawkey kbhawkey reopened this Dec 31, 2019
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Details 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

@kbhawkey
Copy link
Copy Markdown
Contributor Author

I plan to review and clean up the changes. If a new k8s/website theme arrives soon, I will update the related PR and output.

@kbhawkey
Copy link
Copy Markdown
Contributor Author

kbhawkey commented Jan 7, 2020

Seems like this reference could be updated to use HtmlWriter and avoid generating markdown (pare down the UI and cleanup).
Though, the UI would remain unique compared to the main kubectl page (which refers to the reference sections). Not sure which way is easier to maintain.
Closing this PR (again).

@kbhawkey kbhawkey closed this Jan 7, 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/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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