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.

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

add documentation for running knative on IBM s390x/IBM ppc64le platforms #6043

Merged
merged 13 commits into from
Jul 21, 2024
Merged

add documentation for running knative on IBM s390x/IBM ppc64le platforms #6043

merged 13 commits into from
Jul 21, 2024

Conversation

dilipgb
Copy link
Contributor

@dilipgb dilipgb commented Jul 8, 2024

This document updates the installation steps needed to install knative on IBM s390x and IBM ppc64le platforms.

@knative-prow knative-prow bot requested review from Cali0707 and skonto July 8, 2024 08:06
@knative-prow knative-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 8, 2024
Copy link

netlify bot commented Jul 8, 2024

Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit db5e119
🔍 Latest deploy log https://app.netlify.com/sites/knative/deploys/669d10690e859c0008cd1a26
😎 Deploy Preview https://deploy-preview-6043--knative.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@davidhadas
Copy link
Contributor

I find the doc confusing:

  1. It is unclear how Kourier differ from "3-scale-kourier-gateway"
  2. It is unclear how to merge the instructions provided to the "3-scale-kourier-gateway" with those already shown for Kourier

I would suggest taking a straight forward approach and stating in the Note something like: "Use this alternative documentation to install a networking layer on IBM Z and IBM Power platforms."

Then in the documentation, I would suggest to include all steps needed to get to a working networking layer.

Instead of "Customizing 3-scale-kourier-gateway on IBM Z and IBM P platform" as a header, I would use "Install a networking layer on IBM Z and IBM Power platforms."

Instead of "To run 3-scale-kourier-gateway on IBM Z and IBM P platforms, ..." I suggest "To install a networking layer on IBM Z and IBM P platforms, ..." and spell out the steps to do that

@dilipgb
Copy link
Contributor Author

dilipgb commented Jul 9, 2024

@davidhadas 3-scale-kourier-gateway is the deployment name for kourier gateway deployed as part of the kourier deployment. I used the same deployment name, so users who face issues pulling images can relate to.

I will create a single document instead of redirecting user multiple times to different documentations. But there is no difference between standard deployment and deployment we are suggesting. We are patching the image for the same deployment that is done via standard knative scripts.

@davidhadas
Copy link
Contributor

davidhadas commented Jul 9, 2024

@davidhadas 3-scale-kourier-gateway is the deployment name for kourier gateway deployed as part of the kourier deployment. I used the same deployment name, so users who face issues pulling images can relate to.

I got that , but it is not reflected in the added Knative documentation. I suggest spelling out the information rather than assuming readers will figure it out somehow.

I will create a single document instead of redirecting user multiple times to different documentations. But there is no difference between standard deployment and deployment we are suggesting. We are patching the image for the same deployment that is done via standard knative scripts.

If this is a preliminary step that needs to be taken before continuing to the other steps, you can define it as a preliminary step and not repeat the other steps. In any case, it needs to be very clear to the reader what (s)he should be doing + the order of things.

@dilipgb
Copy link
Contributor Author

dilipgb commented Jul 9, 2024

@davidhadas can you please review and let me know if this looks good? I updated the document to be more readable and why we need to patch the 3-scale-kourier-gateway.

Copy link
Contributor

@davidhadas davidhadas left a comment

Choose a reason for hiding this comment

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

Please consider the below suggested changes.
Please verify that the provided instructions and overall guidance are correct and accurate.

@dilipgb dilipgb requested a review from davidhadas July 15, 2024 13:10
@dilipgb
Copy link
Contributor Author

dilipgb commented Jul 15, 2024

@davidhadas I think sub point in docs/install/yaml-install/serving/install-serving-with-yaml-on-IBM-Z-and-IBM-P.md is not rendering properly in the deploy preview. I will see how to add them in markdown and commit it.

@dilipgb
Copy link
Contributor Author

dilipgb commented Jul 15, 2024

@davidhadas I have corrected the alignment. Can you please review it again?

@davidhadas
Copy link
Contributor

/lgtm
@skonto, @Cali0707 can you pls have a look?

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 21, 2024
@davidhadas davidhadas requested review from davidhadas and removed request for davidhadas July 21, 2024 13:46
Copy link

knative-prow bot commented Jul 21, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidhadas, dilipgb

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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 21, 2024
@knative-prow knative-prow bot merged commit 1bf2543 into knative:main Jul 21, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. 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.

2 participants