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

[UPDATED] the home page svg file #5721

Merged
merged 7 commits into from
Oct 20, 2023
Merged

[UPDATED] the home page svg file #5721

merged 7 commits into from
Oct 20, 2023

Conversation

vinfinity7
Copy link
Contributor

Fixes #5720

The Home page svg file has been changed to new one provided to me 👍

@netlify
Copy link

netlify bot commented Oct 17, 2023

Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 4094905
🔍 Latest deploy log https://app.netlify.com/sites/knative/deploys/65317b8371513b0009ecd064
😎 Deploy Preview https://deploy-preview-5721--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.

@knative-prow knative-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 17, 2023
@knative-prow knative-prow bot requested review from nainaz and pmbanugo October 17, 2023 19:29
Copy link
Member

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

@vinfinity7 you will have to replace the existing home image for the flowchart for this change to work, rather than create a new one. If you look at the site preview for this PR (https://deploy-preview-5721--knative.netlify.app/docs/), it is still showing the old image.

@vinfinity7
Copy link
Contributor Author

@Cali0707 please review the changes

Copy link
Member

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

@vinfinity7 the file overrides/images/home-images/knative_flowchart_graphic.svg is still here, even though it looks like you've deleted it's contents. Can you delete it fully?

Also, you have been adding a new file, when what this issue is asking you to do is to replace the existing file. Can you find the file that is currently being used in the homepage and replace it, rather than adding any new files and/or changing the src of the img tag?

@vinfinity7
Copy link
Contributor Author

@Cali0707 you might wanna lead me on this !

@Cali0707
Copy link
Member

@vinfinity7 if you look here: https://github.com/knative/docs/tree/main/docs/images/home-images you will see that there already is a file with the old image. What you should do is replace that file with the new one, rather than creating a new file. Thanks to git, we will always be able to go back and find the old file if needed.

@knative-prow knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 19, 2023
@vinfinity7
Copy link
Contributor Author

image
Replaced all three instances of the image 👍 @Cali0707

overrides/home.html Outdated Show resolved Hide resolved
Co-authored-by: Calum Murray <[email protected]>
@vinfinity7
Copy link
Contributor Author

@Cali0707 why is the Knative Verify Build test failing ?

@Cali0707
Copy link
Member

@Cali0707 why is the Knative Verify Build test failing ?

This is an unrelated issue to your changes, I wouldn't worry about it

@Cali0707
Copy link
Member

/lgtm
/cc @mmejia02 for a final approval

@knative-prow knative-prow bot requested a review from mmejia02 October 19, 2023 19:07
@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 19, 2023
@knative knative deleted a comment from knative-prow bot Oct 19, 2023
@mmejia02
Copy link

/approve

@knative-prow
Copy link

knative-prow bot commented Oct 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mmejia02, vinfinity7

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 Oct 20, 2023
@knative-prow knative-prow bot merged commit 9bef313 into knative:main Oct 20, 2023
15 of 16 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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change homepage diagram
3 participants