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

Match API reference to version of Kubernetes we're documenting #25552

Merged
merged 2 commits into from
Dec 11, 2020
Merged

Match API reference to version of Kubernetes we're documenting #25552

merged 2 commits into from
Dec 11, 2020

Conversation

sftim
Copy link
Contributor

@sftim sftim commented Dec 10, 2020

Once backported appropriately, this could be a fix for issue #25548

@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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 10, 2020
@sftim
Copy link
Contributor Author

sftim commented Dec 10, 2020

@kbhawkey do you think this is an appropriate fix? Also, @zacharysarah I'd welcome your thoughts as you know the historical context here.

@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Dec 10, 2020
@netlify
Copy link

netlify bot commented Dec 10, 2020 •
edited
Loading

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 7727cce

https://deploy-preview-25552--kubernetes-io-master-staging.netlify.app

@zacharysarah
Copy link
Contributor

@sftim The implementation looks solid in concept. I do notice that the "v" is already baked into the variable value.

https://5fd2ad7d6be32500089fbd14--kubernetes-io-master-staging.netlify.app/docs/reference/

Screen Shot 2020-12-10 at 3 58 57 PM

kbhawkey
kbhawkey reviewed Dec 11, 2020
View reviewed changes
content/en/docs/reference/_index.md Outdated
@@ -18,7 +18,7 @@ This section of the Kubernetes documentation contains references.

## API Reference

* [Kubernetes API Reference {{< latest-version >}}](/docs/reference/generated/kubernetes-api/{{< latest-version >}}/)
* [API Reference for Kubernetes v{{< param "version" >}}](/docs/reference/generated/kubernetes-api/{{< param "version" >}}/)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think version and latest variables are equivalent?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, version is the right param in this context.

Copy link
Contributor

@kbhawkey kbhawkey Dec 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Interesting. I looked at the previous configurations from 1.14 to 1.20.
Need to track down where the shortcode is used to set the current or latest version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like v1.19, v1.18, v1.17 use the latest parameter to generated the API reference links.
I've opened PRs to modify the latest parameter to the deprecated version in these snapshots.
I agree that "version" makes more sense in this context.

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'd expect that latest evaluated to the most recent released Kubernetes version, whatever documentation version you're looking at, whereas version evaluates to the version number for the documentation you are viewing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, though, how is this parameter used in the context of a deprecated version?

@kbhawkey kbhawkey mentioned this pull request Dec 11, 2020
Merged
@sftim sftim marked this pull request as ready for review December 11, 2020 10:13
@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 Dec 11, 2020
@tengqm
Copy link
Contributor

tengqm commented Dec 11, 2020

/lgtm

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

LGTM label has been added.

Git tree hash: bc63cbf6abba52be5681915026bf1ac72d0a3052

@kbhawkey
Copy link
Contributor

@sftim . The change looks good:
https://deploy-preview-25552--kubernetes-io-master-staging.netlify.app/docs/reference/
Noted that the latest-version shortcode is used in other pages.

@kbhawkey
Copy link
Contributor

kbhawkey commented Dec 11, 2020 •
edited
Loading

Note: site.Params.latest is used in these shortcodes:
latest-version
latest-semver
latest-release-notes
reference-docs (? from release-1.17)
release-branch (? from release-1.17)

@kbhawkey
Copy link
Contributor

/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 Dec 11, 2020
@k8s-ci-robot k8s-ci-robot merged commit a4a78f4 into kubernetes:master Dec 11, 2020
@sftim sftim deleted the 20201210_match_api_reference_to_version branch December 11, 2020 16:27
@sftim
Copy link
Contributor Author

sftim commented Dec 11, 2020

I'll PR a backport.

@sftim sftim mentioned this pull request Dec 11, 2020
Merged
@sftim sftim mentioned this pull request Dec 11, 2020
Merged
@kbhawkey
Copy link
Contributor

I may have covered these changes in my recent PRs.

@Arhell Arhell mentioned this pull request Dec 11, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tengqm tengqm tengqm left review comments

@kbhawkey kbhawkey kbhawkey left review comments

@daminisatya daminisatya Awaiting requested review from daminisatya

@steveperry-53 steveperry-53 Awaiting requested review from steveperry-53

Assignees

@tengqm tengqm

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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sftim @zacharysarah @tengqm @k8s-ci-robot @kbhawkey