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 the requirements for template and list resources for infrastructure providers #5371

Merged
merged 1 commit into from
Oct 6, 2021
Merged

📖 Document the requirements for template and list resources for infrastructure providers #5371

merged 1 commit into from
Oct 6, 2021

Conversation

randomvariable
Copy link
Member

Signed-off-by: Naadir Jeewa [email protected]

What this PR does / why we need it:

Turns out only CAPV had ObjectMeta fields on the template resources, but makes sense for all providers to do so, so making this explicit in the docs.

Part of #5240

/kind documentation
/area api
/priority important-soon

@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. area/api Issues or PRs related to the APIs priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 1, 2021
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 1, 2021
@randomvariable randomvariable changed the title 📖 Document the requirements for template and list resources for infrastructure providers. 📖 Document the requirements for template and list resources for infrastructure providers Oct 1, 2021
@fabriziopandini
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 4, 2021
sbueringer
sbueringer reviewed Oct 4, 2021
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.

Thx for taking care of this!
Should we create a follow-up issue to implement it in CAPD?

docs/book/src/developer/providers/cluster-infrastructure.md Outdated Show resolved Hide resolved
docs/book/src/developer/providers/cluster-infrastructure.md Outdated Show resolved Hide resolved
@vincepri
Copy link
Member

vincepri commented Oct 5, 2021

@randomvariable do you have time to get the comments above addressed?

@randomvariable randomvariable mentioned this pull request Oct 5, 2021
Closed
@randomvariable randomvariable force-pushed the document-list-and-template-resources branch from 802a230 to 6c80808 Compare October 5, 2021 23:21
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 5, 2021
@randomvariable randomvariable force-pushed the document-list-and-template-resources branch from 6c80808 to 17e9503 Compare October 5, 2021 23:27
@randomvariable
Copy link
Member Author

@vincepri Done

@randomvariable
Copy link
Member Author

(albeit with a follow up issue to write the control-plane doc).

@randomvariable randomvariable mentioned this pull request Oct 6, 2021
Closed
sbueringer
sbueringer reviewed Oct 6, 2021
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.

Last nits from my side.

Thank you very much for this documentation! :)

docs/book/src/developer/providers/bootstrap.md Outdated Show resolved Hide resolved
docs/book/src/developer/providers/bootstrap.md Outdated Show resolved Hide resolved
docs/book/src/developer/providers/cluster-infrastructure.md Show resolved Hide resolved
docs/book/src/developer/providers/v1alpha4-to-v1beta1.md Outdated Show resolved Hide resolved
@randomvariable randomvariable force-pushed the document-list-and-template-resources branch from 2c2a62e to ab01312 Compare October 6, 2021 08:13
@sbueringer sbueringer mentioned this pull request Oct 6, 2021
Closed
vincepri
vincepri approved these changes Oct 6, 2021
View reviewed changes
Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2021
@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 Oct 6, 2021
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 6, 2021
@randomvariable
Copy link
Member Author

argh

Document the requirements for template and list resources for
6efbb03 
infrastructure providers.

Signed-off-by: Naadir Jeewa <[email protected]>
@randomvariable randomvariable force-pushed the document-list-and-template-resources branch from ab01312 to 6efbb03 Compare October 6, 2021 16:17
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 6, 2021
@randomvariable
Copy link
Member Author

rebased

@sbueringer
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 6, 2021
@k8s-ci-robot k8s-ci-robot merged commit b96e514 into kubernetes-sigs:main Oct 6, 2021
@k8s-ci-robot k8s-ci-robot added this to the v0.4 milestone Oct 6, 2021
@jsturtevant jsturtevant mentioned this pull request Oct 15, 2021
Merged
3 tasks
@killianmuldoon killianmuldoon mentioned this pull request Jul 25, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabriziopandini fabriziopandini fabriziopandini left review comments

@sbueringer sbueringer sbueringer left review comments

@vincepri vincepri vincepri approved these changes

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

Assignees

@vincepri vincepri

@sbueringer sbueringer

@fabriziopandini fabriziopandini

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/api Issues or PRs related to the APIs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v0.4
Development

Successfully merging this pull request may close these issues.
5 participants
@randomvariable @fabriziopandini @vincepri @k8s-ci-robot @sbueringer