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

Documentation refactor #5088

Merged
merged 1 commit into from
Aug 24, 2024
Merged

Documentation refactor #5088

merged 1 commit into from
Aug 24, 2024

Conversation

dtzar
Copy link
Contributor

@dtzar dtzar commented Aug 20, 2024

What type of PR is this?
/kind documentation

What this PR does / why we need it:
The documentation is very confusing as to what parts relate to self-managed versus managed clusters. Furthermore, it isn't clear there are two ways to provision AKS clusters nor the differences between the options.

Refactor documentation

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 20, 2024
@dtzar dtzar requested a review from willie-yao August 20, 2024 23:12
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 20, 2024
@codecov Codecov
Copy link

codecov bot commented Aug 20, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.18%. Comparing base (a606de0) to head (91130bd).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #5088   +/-   ##
=======================================
  Coverage   51.18%   51.18%           
=======================================
  Files         273      273           
  Lines       24611    24611           
=======================================
  Hits        12597    12597           
  Misses      11229    11229           
  Partials      785      785

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dtzar
Copy link
Contributor Author

dtzar commented Aug 21, 2024

@mbrow137 appreciate it if you can review

mbrow137
mbrow137 suggested changes Aug 21, 2024
View reviewed changes
Copy link
Contributor

@mbrow137 mbrow137 left a comment

Choose a reason for hiding this comment

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

Line 17, what supports the preview API?

mbrow137
mbrow137 suggested changes Aug 21, 2024
View reviewed changes
Copy link
Contributor

@mbrow137 mbrow137 left a comment

Choose a reason for hiding this comment

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

Line 125: You must*

mbrow137
mbrow137 approved these changes Aug 22, 2024
View reviewed changes
docs/book/src/managed/managed.md Outdated Show resolved Hide resolved
docs/book/src/developers/development.md Outdated Show resolved Hide resolved
@dtzar dtzar mentioned this pull request Aug 23, 2024
Merged
willie-yao
willie-yao reviewed Aug 23, 2024
View reviewed changes
Copy link
Contributor

@willie-yao willie-yao left a comment

Choose a reason for hiding this comment

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

Looks good overall! Some minor nits

docs/book/src/getting-started.md
@@ -45,7 +45,7 @@ An Azure Service Principal is needed for deploying Azure resources. The below in
```

5. Create an Azure Service Principal by running the following command or skip this step and use a previously created Azure Service Principal.
NOTE: the "owner" role is required to be able to create role assignments for [system-assigned managed identity](vm-identity.md).
NOTE: the "owner" role is required to be able to create role assignments for system-assigned managed identity.
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this still link to vm-identity.md?

Copy link
Contributor Author

@dtzar dtzar Aug 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

When I looked at this article, I thought it was misleading since it was specific to self-managed. I also plan to entirely overhaul the getting started, so I thought it was best to just remove the link.

docs/book/src/managed/managed.md Outdated Show resolved Hide resolved
docs/book/src/managed/managedcluster-joinVMSS.md Outdated Show resolved Hide resolved
willie-yao
willie-yao approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@willie-yao willie-yao left a comment

Choose a reason for hiding this comment

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

/lgtm
/hold for squash

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 23, 2024
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 23, 2024
@willie-yao
Copy link
Contributor

/lgtm cancel
Whoops missed some things

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 23, 2024
willie-yao
willie-yao reviewed Aug 23, 2024
View reviewed changes
docs/book/src/SUMMARY.md Outdated Show resolved Hide resolved
docs/book/src/managed/managed.md Outdated Show resolved Hide resolved
@dtzar dtzar force-pushed the refactorbook branch 2 times, most recently from f2e0be1 to a2fd01b Compare August 23, 2024 23:20
@dtzar
Copy link
Contributor Author

dtzar commented Aug 23, 2024

/label tide/merge-method-squash

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 23, 2024
@dtzar dtzar force-pushed the refactorbook branch 3 times, most recently from abde2c0 to 7e84e0b Compare August 23, 2024 23:47
willie-yao
willie-yao approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@willie-yao willie-yao left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Aug 23, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 8967b3fc2ce9bb2d4305499afc7a4840e7422592

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: willie-yao

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 Aug 23, 2024
@k8s-ci-robot k8s-ci-robot merged commit c4a3065 into kubernetes-sigs:main Aug 24, 2024
18 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Aug 24, 2024
@dtzar dtzar deleted the refactorbook branch August 24, 2024 00:31
@willie-yao
Copy link
Contributor

@jackfrancis Would you be able to do a netfliy release for the docs?

@jackfrancis
Copy link
Contributor

@willie-yao Netlify sources documentation from a branch. It's currently configured to release-1.16, so once we cherry-pick these changes into that branch we can re-publish.

@dtzar I assume there are no documentation updates that reflect new foo that is in main (and isn't in the v1.16.0 release)?

@dtzar
Copy link
Contributor Author

dtzar commented Aug 26, 2024

Correct

@jackfrancis
Copy link
Contributor

/cherry-pick release-1.16

@k8s-infra-cherrypick-robot
Copy link

@jackfrancis: new pull request created: #5094

In response to this:

/cherry-pick release-1.16

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-sigs/prow repository.

@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot mentioned this pull request Aug 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrow137 mbrow137 mbrow137 approved these changes

@willie-yao willie-yao willie-yao approved these changes

@jsturtevant jsturtevant Awaiting requested review from jsturtevant

@mboersma mboersma Awaiting requested review from mboersma

Assignees

@willie-yao willie-yao

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. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
CAPZ Planning
Archived in project
Milestone
v1.17
Development

Successfully merging this pull request may close these issues.
6 participants
@dtzar @willie-yao @k8s-ci-robot @jackfrancis @k8s-infra-cherrypick-robot @mbrow137