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 how to change a ClusterClass #5978

Merged
merged 1 commit into from
Feb 1, 2022
Merged

📖 Document how to change a ClusterClass #5978

merged 1 commit into from
Feb 1, 2022

Conversation

fabriziopandini
Copy link
Member

What this PR does / why we need it:
Improving documentation on how to change a ClusterClass

Which issue(s) this PR fixes:
Rif #5953

@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/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 25, 2022
@fabriziopandini
Copy link
Member Author

@killianmuldoon @sbueringer @ykakarap asking for the first round of feedback in order to understand if the content fits properly into the documentation plan being worked in parallel by all of us

@sbueringer sbueringer mentioned this pull request Jan 25, 2022
Closed
7 tasks
@fabriziopandini fabriziopandini changed the title [WIP] 📖 Document how to change a ClusterClass 📖 Document how to change a ClusterClass Jan 26, 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 Jan 26, 2022
sbueringer
sbueringer reviewed Jan 27, 2022
View reviewed changes
Copy link
Member

@sbueringer sbueringer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

All nits without impact on the structure. Really great work!!

docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
sbueringer
sbueringer reviewed Jan 28, 2022
View reviewed changes
Copy link
Member

@sbueringer sbueringer left a comment

Choose a reason for hiding this comment

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

A few nits, otherwise perfect :)

docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
@fabriziopandini fabriziopandini force-pushed the document-changing-clusterclass branch from a380d1f to 72450d4 Compare January 28, 2022 13:58
@sbueringer
Copy link
Member

Thx. Again, great work! :)

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2022
killianmuldoon
killianmuldoon reviewed Jan 28, 2022
View reviewed changes
Copy link
Contributor

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

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

Just nits - this doc is looking great!

docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
ykakarap
ykakarap suggested changes Jan 31, 2022
View reviewed changes
Copy link
Contributor

@ykakarap ykakarap left a comment

Choose a reason for hiding this comment

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

Just a few nits. Overall this docs looks great!

docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
docs/book/src/tasks/experimental-features/cluster-class/change-clusterclass.md Outdated Show resolved Hide resolved
@fabriziopandini fabriziopandini force-pushed the document-changing-clusterclass branch from 72450d4 to 4d0a8ce Compare January 31, 2022 17:13
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2022
@ykakarap
Copy link
Contributor

LGTM pending the broken links fix.
Looks like the links are broken because this PR renamed a few links: #6018

Rebasing the PR on main should fix the problem.

@killianmuldoon killianmuldoon mentioned this pull request Jan 31, 2022
Merged
@fabriziopandini fabriziopandini force-pushed the document-changing-clusterclass branch 2 times, most recently from 40ca087 to 1fa4075 Compare January 31, 2022 19:37
@vincepri
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vincepri

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 Jan 31, 2022
@sbueringer
Copy link
Member

@fabriziopandini last nit. I think we can merge afterwards

@sbueringer
Copy link
Member

/cherry-pick release-1.1

@k8s-infra-cherrypick-robot
Copy link

@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.1 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.1

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.

@fabriziopandini fabriziopandini force-pushed the document-changing-clusterclass branch from 1fa4075 to bee8d4f Compare February 1, 2022 09:03
@fabriziopandini
Copy link
Member Author

@sbueringer done!

@sbueringer
Copy link
Member

Thx!
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 1, 2022
@k8s-ci-robot k8s-ci-robot merged commit 236e6da into kubernetes-sigs:main Feb 1, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.2 milestone Feb 1, 2022
@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot mentioned this pull request Feb 1, 2022
Merged
@k8s-infra-cherrypick-robot
Copy link

@sbueringer: new pull request created: #6031

In response to this:

/cherry-pick release-1.1

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.

@fabriziopandini fabriziopandini deleted the document-changing-clusterclass branch February 3, 2022 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@killianmuldoon killianmuldoon killianmuldoon left review comments

@sbueringer sbueringer sbueringer left review comments

@ykakarap ykakarap ykakarap requested changes

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

Assignees

@sbueringer sbueringer

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.
7 participants
@fabriziopandini @sbueringer @ykakarap @vincepri @k8s-ci-robot @k8s-infra-cherrypick-robot @killianmuldoon