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

OSDOCS-10419_2#adding file content type #75866

Merged
merged 1 commit into from May 20, 2024
Merged

OSDOCS-10419_2#adding file content type #75866

merged 1 commit into from May 20, 2024
+5 −0

Conversation

brendan-daly-red-hat
Copy link

@brendan-daly-red-hat brendan-daly-red-hat commented May 13, 2024 •
edited

Version:
4.16

Issue:
#75682 (The :_mod-docs-content-type: module file metadata is missing for some modules)

Link to docs preview:
N/a

QE review:

  • QE has approved this change.

Additional information:

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 13, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented May 13, 2024 •
edited

🤖 Fri May 17 15:19:18 - Prow CI generated the docs preview:

https://75866--ocpdocs-pr.netlify.app/openshift-enterprise/latest/installing/installing_gcp/installing-gcp-account.html
https://75866--ocpdocs-pr.netlify.app/openshift-enterprise/latest/installing/installing_gcp/installing-gcp-user-infra.html
https://75866--ocpdocs-pr.netlify.app/openshift-enterprise/latest/installing/installing_gcp/installing-gcp-user-infra-vpc.html
https://75866--ocpdocs-pr.netlify.app/openshift-enterprise/latest/installing/installing_gcp/installing-restricted-networks-gcp.html

@brendan-daly-red-hat
Copy link
Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label May 14, 2024
@maxwelldb maxwelldb added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels May 14, 2024
@maxwelldb maxwelldb self-requested a review May 14, 2024 13:02
@maxwelldb maxwelldb added this to the Planned for 4.16 GA milestone May 14, 2024
maxwelldb
maxwelldb approved these changes May 14, 2024
View reviewed changes
Copy link
Contributor

@maxwelldb maxwelldb left a comment

Choose a reason for hiding this comment

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

The line between concept and reference here isn't totally clear to me. Ex: "Required GCP roles" is a concept, but "Required GCP permissions for shared VPC installations" isn't?

Otherwise, lgtm.

@maxwelldb maxwelldb added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels May 14, 2024
@brendan-daly-red-hat brendan-daly-red-hat force-pushed the OSDOCS-10419_2 branch 2 times, most recently from 9faae29 to ca77868 Compare May 14, 2024 14:55
@brendan-daly-red-hat
Copy link
Author

@maxwelldb, Thanks for the review. I changed 'Required GCP permissions for shared VPC installations' to a concept. Can you take a quick look?

maxwelldb
maxwelldb reviewed May 14, 2024
View reviewed changes
Copy link
Contributor

@maxwelldb maxwelldb left a comment

Choose a reason for hiding this comment

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

Left my thoughts on what these things are, but I'm just a peer reviewer and this is all advisory. :)

modules/minimum-required-permissions-ipi-gcp-xpn.adoc Outdated
@@ -2,6 +2,7 @@
//
// * installing/installing_gcp/installing-gcp-account.adoc

:_mod-docs-content-type: CONCEPT
Copy link
Contributor

Choose a reason for hiding this comment

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

This module doesn't really fit into any of the mod docs types as it's written, IMO. Since it's saying that the reader must do something in the course of a task flow and has that "you can skip this section part," it resembles a procedure, though it's not in that format.

https://redhat-documentation.github.io/modular-docs/#_creating_modules

Copy link
Contributor

Choose a reason for hiding this comment

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

Note - while this module doesn't quite fit into any of the mod doc types, we will look to restructure it in a future update. For now the module will be categorized as a procedure.

modules/installation-gcp-regions.adoc
@@ -4,6 +4,7 @@
// * installing/installing_gcp/installing-gcp-user-infra.adoc
// * installing/installing_gcp/installing-restricted-networks-gcp.adoc

:_mod-docs-content-type: REFERENCE
Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely a reference.

modules/installation-gcp-limits.adoc Outdated
@@ -14,6 +14,7 @@ ifeval::["{context}" == "installing-restricted-networks-gcp"]
:template:
endif::[]

:_mod-docs-content-type: CONCEPT
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a reference to me.

@brendan-daly-red-hat brendan-daly-red-hat force-pushed the OSDOCS-10419_2 branch 7 times, most recently from 31492eb to baca4c0 Compare May 17, 2024 13:57
@openshift-ci openshift-ci bot 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 May 17, 2024
@openshift-ci openshift-ci bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 17, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented May 17, 2024

@brendan-daly-red-hat: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@brendan-daly-red-hat
Copy link
Author

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label May 20, 2024
@michaelryanpeter
Copy link
Contributor

/label merge-review-in-progress

@openshift-ci openshift-ci bot added the merge-review-in-progress Signifies that the merge review team is reviewing this PR label May 20, 2024
michaelryanpeter
michaelryanpeter reviewed May 20, 2024
View reviewed changes
Copy link
Contributor

@michaelryanpeter michaelryanpeter left a comment

Choose a reason for hiding this comment

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

I have reservations about this module being labelled as a procedure, but after chatting with @bscott-rh, I know you are going to work on restructuring this module soon to either conform with either the procedure or reference format.

@michaelryanpeter michaelryanpeter merged commit 200975b into openshift:main May 20, 2024
3 checks passed
@michaelryanpeter
Copy link
Contributor

/cherrypick enterprise-4.16

@openshift-cherrypick-robot
Copy link

@michaelryanpeter: new pull request created: #76253

In response to this:

/cherrypick enterprise-4.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.

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

@maxwelldb maxwelldb maxwelldb approved these changes

@michaelryanpeter michaelryanpeter michaelryanpeter left review comments

@bscott-rh bscott-rh bscott-rh left review comments

Assignees
No one assigned
Labels
branch/enterprise-4.16 merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
Planned for 4.16 GA
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@brendan-daly-red-hat @ocpdocs-previewbot @michaelryanpeter @openshift-cherrypick-robot @maxwelldb @bscott-rh