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

Fixing build broken links for netlify build (remove from todoPatterns) #3963

Merged
merged 5 commits into from
Jan 29, 2025

Conversation

jaiakash
Copy link
Contributor

@jaiakash jaiakash commented Jan 18, 2025

Hi, I removed all (for now yes, all) links from todoPatterns to check the status of netlify build.

Update 1: When i removed all the links total 6 tests were failing.
Screenshot 2025-01-18 at 12 42 51 PM

After reverting those link patterns, build is passing.

Fixes #3960

Copy link

Hi @jaiakash. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

Copy link
Member

@Arhell Arhell left a comment

Choose a reason for hiding this comment

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

/ok-to-test

Copy link
Contributor

@hbelmiro hbelmiro left a comment

Choose a reason for hiding this comment

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

@jaiakash Have you verified where the pending links are? Please consider using skipPatterns as mentioned by Mathew in the issue.

NOTE: if some of the TODO links are actually correct but the plugin is making a mistake, we can use the skipPatterns section instead, so they don't appear in the logs.

@jaiakash
Copy link
Contributor Author

jaiakash commented Jan 18, 2025

Hi @hbelmiro, So I tried to dig deeper into the issue. There are 4 patterns that are causing issues right now. I will go through one by one.

  1. public/docs/started/getting-started
    This is the legacy page for getting started/installation for KubeFlow. Currently, 2 places ( first is here and 2nd is here ) we are using this link.

Solution: We can replace it with KubeFlow installation link.

  1. public/docs/distributions/gke/
    I think these pages are permanently moved or deleted for the current version of docs. There is no occurrence of this pattern in new docs as relative path, only legacy-v1 has these links as relative path. New docs uses absolute path for GKE. Check below line
    check [authentication-pipelines](https://www.kubeflow.org/docs/distributions/gke/pipelines/authentication-pipelines/)

Solution: We have to use GKE docs absolute link for referencing above pattern. GKE Docs

@jaiakash
Copy link
Contributor Author

jaiakash commented Jan 18, 2025

  1. public/docs/other-guides
    I did not understand what this other guide meant in docs. The link itself is broken on the production webssite. Click on Further setup and troubleshooting on this page https://www.kubeflow.org/docs/started/support/#support-from-the-kubeflow-community
    Let me know what this link is supposed to do.

  2. public/docs/examples/shared-resources
    This pattern exists only in legacy-v1 docs only. I think this link to supposed to redirect to the examples page.

Solution: Replace this link with examples page (https://www.kubeflow.org/docs/started/kubeflow-examples/)

@jaiakash
Copy link
Contributor Author

Hi @hbelmiro @thesuperzapper, can you please review and give your suggestions?

@hbelmiro
Copy link
Contributor

@jaiakash for [3], I suggest removing that bullet.
All the other solutions proposed sound good to me.

@google-oss-prow google-oss-prow bot added size/M and removed size/S labels Jan 20, 2025
Signed-off-by: Akash Jaiswal <[email protected]>
@jaiakash
Copy link
Contributor Author

Hi @hbelmiro ,

  1. public/docs/started/getting-started
    This is the legacy page for getting started/installation for KubeFlow. Currently, 2 places ( first is here and 2nd is here ) we are using this link.

Solution: We can replace it with KubeFlow installation link.

  1. public/docs/distributions/gke/
    I think these pages are permanently moved or deleted for the current version of docs. There is no occurrence of this pattern in new docs as relative path, only legacy-v1 has these links as relative path. New docs uses absolute path for GKE.
    Solution: We have to use GKE docs absolute link for referencing above pattern. GKE Docs

For 1, I changed the link to KubeFlow installation docs.
For 2, I tried using Kubeflow link and then redirecting to GKE docs, but still the tests were failing. So I changed to actual GKE docs links.

  1. public/docs/other-guides
    I did not understand what this other guide meant in docs. The link itself is broken on the production webssite. Click on Further setup and troubleshooting on this page kubeflow.org/docs/started/support#support-from-the-kubeflow-community
    Let me know what this link is supposed to do.
  2. public/docs/examples/shared-resources
    This pattern exists only in legacy-v1 docs only. I think this link to supposed to redirect to the examples page.

Solution: Replace this link with examples page (kubeflow.org/docs/started/kubeflow-examples)

For 3, I deleted it since its not needed now.

For 4, shared resources are supposed to point at examples but not for all the pages, so I moved it to skipPatterns

Copy link
Contributor

@hbelmiro hbelmiro left a comment

Choose a reason for hiding this comment

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

/lgtm

@jaiakash
Copy link
Contributor Author

Any update on PR @hbelmiro ?

@hbelmiro
Copy link
Contributor

@andreyvelich @terrytangyuan can one of you guys PTAL?

Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hbelmiro, terrytangyuan

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

@google-oss-prow google-oss-prow bot merged commit 06b1b77 into kubeflow:master Jan 29, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix remaining broken links (remove from todoPatterns)
4 participants