Skip to content

Conversation

@kalexand-rh kalexand-rh added this to the Next Release milestone Jul 28, 2021
@openshift-ci openshift-ci bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 28, 2021
@netlify
Copy link

netlify bot commented Jul 28, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: b533987

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/6101b85f7e2c700007e6f016

😎 Browse the preview: https://deploy-preview-34976--osdocs.netlify.app

@kalexand-rh
Copy link
Contributor Author

@tsze-redhat, will you PTAL? It's easier to track the important part of this change by checking out the preview.

@tsze-redhat
Copy link

Sorry I didn't notice this before.

Looking at https://deploy-preview-34976--osdocs.netlify.app/openshift-enterprise/latest/installing/installing_gcp/installing-restricted-networks-gcp.html#installation-initializing_installing-restricted-networks-gcp,

step 2 c has
Edit the install-config.yaml file to provide the additional information that is required for an installation in a restricted network.

Do you know where that comes from? It isn't a necessary step, I think. @wking Correct me here please.

Other than the above, LGTM.

Thanks.

@kalexand-rh
Copy link
Contributor Author

Sorry I didn't notice this before.

Looking at https://deploy-preview-34976--osdocs.netlify.app/openshift-enterprise/latest/installing/installing_gcp/installing-restricted-networks-gcp.html#installation-initializing_installing-restricted-networks-gcp,

step 2 c has
Edit the install-config.yaml file to provide the additional information that is required for an installation in a restricted network.

Do you know where that comes from? It isn't a necessary step, I think. @wking Correct me here please.

Other than the above, LGTM.

Thanks.

@tsze-redhat, it's applying the same GCP + restricted customizations that you approved here.

@tsze-redhat
Copy link

and I asked the same questions there too. :)

Sorry.

LGTM

Thanks.

@kalexand-rh
Copy link
Contributor Author

and I asked the same questions there too. :)

Sorry.

LGTM

Thanks.

Thank you for taking a look! I am glad that you didn't have a new concern about this specific implementation and that it looks good. :)

@kalexand-rh kalexand-rh added the peer-review-needed Signifies that the peer review team needs to review this PR label Aug 19, 2021
Copy link
Contributor

@EricPonvelle EricPonvelle left a comment

Choose a reason for hiding this comment

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

/lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.6 branch/enterprise-4.7 branch/enterprise-4.8 branch/enterprise-4.9 lgtm Indicates that a PR is ready to be merged. peer-review-needed Signifies that the peer review team needs to review this PR size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants