Skip to content

update-imported-docs in golang#6642

Merged
steveperry-53 merged 1 commit intokubernetes:masterfrom
mengqiy:play
Jan 3, 2018
Merged

update-imported-docs in golang#6642
steveperry-53 merged 1 commit intokubernetes:masterfrom
mengqiy:play

Conversation

@mengqiy
Copy link
Member

@mengqiy mengqiy commented Dec 11, 2017

update-imported-docs/config.yaml should have the src and dest paths for the target files.

fixes: #4283

/assign @steveperry-53

I'm not sure what branch (master or release-1.9) should this PR target.


This change is Reviewable

@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 Dec 11, 2017
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Dec 11, 2017

Deploy preview ready!

Built with commit c22066e

https://deploy-preview-6642--kubernetes-io-master-staging.netlify.com

@mengqiy
Copy link
Member Author

mengqiy commented Dec 11, 2017

/assign @apelisse

@steveperry-53
Copy link
Contributor

@mengqiy The master branch is fine for this PR.

@mengqiy
Copy link
Member Author

mengqiy commented Dec 12, 2017

@steveperry-53 Are you going to use this script to update the imported docs for 1.9 release? Or if you want to keep the PRs that are manually crafted i.e. #6614 #6619 #6628?

@steveperry-53
Copy link
Contributor

steveperry-53 commented Dec 12, 2017

@mengqiy I was thinking I would use this script, and make a new PR to replace #6614, #6619, and #6628. Does that sound reasonable? I'm running the script to do that right now.

@mengqiy
Copy link
Member Author

mengqiy commented Dec 12, 2017

SG. I guess you only need to run it once after you populate the config.yaml. IOW, I guess you will only need 1 PR.

One concern: if this PR is merged into master branch, will you guys do fast-forward (or something else) to make 1.9 branch have this program as well?

#6614, #6619, and #6628 can be used as reference to check if everything looks correct.

@steveperry-53
Copy link
Contributor

steveperry-53 commented Dec 12, 2017

@mengqiy At this point, we should wait until after the 1.9 release to merge this PR. On Wednesday, 12/13, the plan is to merge kubernetes/website:release-1.9 into kubernetes/website:master. After that, we can merge this PR into master. In the meantime, I have cloned mengqiy/website:play, and am running the script to generate docs. I'll submit those generated docs in a PR to kubernetes/website:release-1.9. I should be able to do that tonight.

@steveperry-53
Copy link
Contributor

steveperry-53 commented Dec 12, 2017

See #6658. Notice that in some topics, the synopsis appears twice. Example: kubectl options. For the 1.9 release, I'll just fix these manually.

@mengqiy
Copy link
Member Author

mengqiy commented Dec 12, 2017

Thanks for you feedback.
Can you provide me the config.yaml?
So I can reproduce it and check why it happens. And then fix it.

@steveperry-53
Copy link
Contributor

repos:
- name: kubernetes
  remote: https://github.com/kubernetes/kubernetes.git
  branch: release-1.9
  files:
  - src: docs/admin/cloud-controller-manager.md
    dst: docs/reference/generated/cloud-controller-manager.md
  - src: docs/admin/kube-apiserver.md
    dst: docs/reference/generated/kube-apiserver.md
  - src: docs/admin/kube-controller-manager.md
    dst: docs/reference/generated/kube-controller-manager.md
  - src: docs/admin/kubelet
    dst: docs/reference/generated/kubelet.md
  - src: docs/admin/kube-proxy.md
    dst: docs/reference/generated/kube-proxy.md
  - src: docs/admin/kube-scheduler.md
    dst: docs/reference/generated/kube-scheduler.md
  - src: docs/user-guide/kubectl/kubectl.md
    dst: docs/reference/generated/kubectl/kubectl.md
- name: federation
  remote: https://github.com/kubernetes/federation.git
#  # Change this to a release branch when federation has release branches.
  branch: master
  files:
  - src: docs/admin/federation-apiserver.md
    dst: docs/reference/generated/federation-apiserver.md
  - src: docs/admin/federation-controller-manager.md
    dst: docs/reference/generated/federation-controller-manager.md
  - src: docs/admin/kubefed_init.md
    dst: docs/reference/generated/kubefed_init.md
  - src: docs/admin/kubefed_join.md
    dst: docs/reference/generated/kubefed_join.md
  - src: docs/admin/kubefed.md
    dst: docs/reference/generated/kubefed.md
  - src: docs/admin/kubefed_options.md
    dst: docs/reference/generated/kubefed_options.md
  - src: docs/admin/kubefed_unjoin.md
    dst: docs/reference/generated/kubefed_unjoin.md
  - src: docs/admin/kubefed_version.md
    dst: docs/reference/generated/kubefed_version.md

@mengqiy
Copy link
Member Author

mengqiy commented Dec 15, 2017

Thanks! I will dig it tomorrow or next Monday.

@mengqiy
Copy link
Member Author

mengqiy commented Dec 22, 2017

See #6658. Notice that in some topics, the synopsis appears twice.

Do you mean it appears once in synopsis and once in Example?

Example: kubectl options.

The link you gave actually links to kubefed options.

After digging this a bit, I found it's not the problem of this script. The generated doc looks the same. So if the format is wrong, then the issue is in the federation repo.

@steveperry-53
Copy link
Contributor

:lgtm:


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@steveperry-53
Copy link
Contributor

steveperry-53 commented Dec 22, 2017

@mengqiy Thanks for checking into this. My mistake on the link text; I meant it to say kubefed options. Are you ready for me to merge this PR now?

These are the files that had the double synopsis:

  • kubectl.md
  • kubefed.md
  • kubefed_init.md
  • kubefed_join.md
  • kubefed_options.md
  • kubefed_unjoin.md
  • kubefed_version.md

The synopsis appeared once just below the title and once just below the Synopsis heading. This commit
shows the files where I removed the synopsis that appeared under the title.

When I get a chance, I'll take a look at the federation scripts, and see about getting those fixed.

Before I did the removal, the pages looked like this:

kubefed init

Initialize a federation control plane

Synopsis

Init initializes a federation control plane.

@mengqiy
Copy link
Member Author

mengqiy commented Jan 2, 2018

kubefed init
Initialize a federation control plane
Synopsis
Init initializes a federation control plane.

kubefed join
Join a cluster to a federation
Synopsis
Join adds a cluster to a federation.

The wording are not the same, so I think they not duplicate but they are generated from different origins with similar meaning.

@steveperry-53
Copy link
Contributor

@mengqiy Should I go ahead and merge this PR?

@mengqiy
Copy link
Member Author

mengqiy commented Jan 3, 2018

Please go ahead and merge it. :)
It should not have any risk.

@steveperry-53 steveperry-53 merged commit 79d24ab into kubernetes:master Jan 3, 2018
@mengqiy mengqiy deleted the play branch January 4, 2018 23:07
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. 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.

Fix broken update-imported-docs.sh

5 participants