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.

Sign up for GitHub

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

Jump to bottom

document kubectl explain openapi v3 #37788

Merged
merged 1 commit into from
Dec 7, 2022
Merged

document kubectl explain openapi v3 #37788

merged 1 commit into from
Dec 7, 2022

Conversation

alexzielenski
Copy link
Contributor

re: kubernetes/enhancements#3515

this enhancement changes the data backing for kubectl explain to use OpenAPIv3 instead of v2 as earlier. It is toggled by en environment variable KUBECTL_EXPLAIN_OPENAPIV3=true.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language labels Nov 8, 2022
@k8s-ci-robot k8s-ci-robot added sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 8, 2022
@alexzielenski alexzielenski mentioned this pull request Nov 8, 2022
Closed
8 tasks
@alexzielenski alexzielenski changed the title kubectl explain placeholder doc kubectl explain openapi v3 placeholder doc Nov 8, 2022
@netlify
Copy link

netlify bot commented Nov 8, 2022

✅ Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 935f0fd
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/636a822b96c1ef0009c91fc7
😎 Deploy Preview https://deploy-preview-37788--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sftim
Copy link
Contributor

sftim commented Nov 8, 2022

/retitle [WIP] document kubectl explain openapi v3

@k8s-ci-robot k8s-ci-robot changed the title kubectl explain openapi v3 placeholder doc [WIP] document kubectl explain openapi v3 Nov 8, 2022
@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 Nov 8, 2022
sftim
sftim reviewed Nov 8, 2022
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

Some early feedback:

Does OpenAPI version switch belong in a cheat sheet? I doubt that many readers look for it in a hurry.

@alexzielenski
Copy link
Contributor Author

Does OpenAPI version switch belong in a cheat sheet? I doubt that many readers look for it in a hurry.

Agreed. Though I did not come across a natural place in the existing docs where I thought it fit. Open to suggestions

@krol3
Copy link
Contributor

krol3 commented Nov 15, 2022

Hi @alexzielenski , Thank you for the draft doc PR here, Please update to Ready for Review, the deadline it's on Tuesday 15th November 2022. Thank you!

@krol3
Copy link
Contributor

krol3 commented Nov 15, 2022

@alexzielenski , Please can you create this PR against the branch 1.26? Here the instructions.

This enhancement is marked as ‘Needs Docs’ for 1.26 release. Please follow the steps detailed in the documentation to open a PR against dev-1.26 branch in the k/website repo. This PR can be just a placeholder at this time, and must be created by November 9. Also, take a look at Documenting for a release to familiarize yourself with the docs requirement for the release.

cc: @cathchu

1 similar comment
@krol3
Copy link
Contributor

krol3 commented Nov 15, 2022

@alexzielenski , Please can you create this PR against the branch 1.26? Here the instructions.

This enhancement is marked as ‘Needs Docs’ for 1.26 release. Please follow the steps detailed in the documentation to open a PR against dev-1.26 branch in the k/website repo. This PR can be just a placeholder at this time, and must be created by November 9. Also, take a look at Documenting for a release to familiarize yourself with the docs requirement for the release.

cc: @cathchu

@alexzielenski alexzielenski changed the base branch from main to dev-1.26 November 15, 2022 19:39
@netlify
Copy link

netlify bot commented Nov 15, 2022

👷 Deploy Preview for kubernetes-io-vnext-staging processing.

Name Link
🔨 Latest commit 65953e4
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-vnext-staging/deploys/6373ec1aa6b24d0008a9ba96

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 15, 2022
@alexzielenski alexzielenski changed the title [WIP] document kubectl explain openapi v3 document kubectl explain openapi v3 Nov 15, 2022
@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 Nov 15, 2022
@alexzielenski alexzielenski requested review from sftim and removed request for krousey and rajeshdeshpande02 November 15, 2022 19:45
@alexzielenski
Copy link
Contributor Author

alexzielenski commented Nov 15, 2022 •
edited
Loading

ive found a more appropriate place for the envar doc, and pointed at the dev-1.26 branch as PR base

sftim
sftim reviewed Nov 29, 2022
View reviewed changes
content/en/docs/reference/kubectl/kubectl.md
Comment on lines +356 to +362
<tr>
<td colspan="2">KUBECTL_EXPLAIN_OPENAPIV3</td>
</tr>
<tr>
<td></td><td style="line-height: 130%; word-wrap: break-word;">Toggles whether calls to `kubectl explain` use the new OpenAPIv3 data source available. OpenAPIV3 is enabled by default since Kubernetes 1.24.
</td>
</tr>
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this change be overwritten by future automatic updates to this page? I'm not sure.

Copy link
Member

Choose a reason for hiding this comment

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

yes, this will be overwritten when the kubectl reference docs are generated.
I'm not exactly sure but it may be better to place this somewhere in
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/kubectl/pkg/cmd/explain/explain.go

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 ok with having this merge now so we can have the change in the history/git log
/approve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reylejano I'd be happy to make the correct change to the generator of this page, but I am having trouble finding it. Could you point to the source for the "envvars" content of this page?

I am basing this work off of prior PR #28692 in which @sftim raised the same concern but the documentation merged without an explicit answer to whether this page is generated or not

sftim
sftim reviewed Nov 29, 2022
View reviewed changes
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

/lgtm

with a reservation

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

LGTM label has been added.

Git tree hash: d0f27a48cb540163b4dbf6861eebae3f2703effd

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: reylejano

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 Dec 7, 2022
@k8s-ci-robot k8s-ci-robot merged commit 0ecc914 into kubernetes:dev-1.26 Dec 7, 2022
@k8s-ci-robot k8s-ci-robot added this to the 1.26 milestone Dec 7, 2022
@ardaguclu ardaguclu mentioned this pull request Apr 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reylejano reylejano reylejano left review comments

@sftim sftim sftim left review comments

Assignees

@sftim sftim

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. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
1.26
Development

Successfully merging this pull request may close these issues.
5 participants
@alexzielenski @sftim @krol3 @k8s-ci-robot @reylejano