-
Notifications
You must be signed in to change notification settings - Fork 29
OCMUI-3556: use "latest" in OCP doc-links #222
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
base: main
Are you sure you want to change the base?
OCMUI-3556: use "latest" in OCP doc-links #222
Conversation
in OCP doc links: - use `latest` for the base link, instead of hard-coding `4.19` - fix path variables; `installing_on_gcp` got replaced with `installing_on_google_cloud`
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReplace hard-coded OCP documentation version with the 'latest' alias in installation links so that documentation URLs always point to the most recent release. Class diagram for updated OCP documentation link constantsclassDiagram
class installLinks {
+SHP_CLI_LATEST : string
+OCP_DOCS_BASE : string
+OSD_DOCS_BASE : string
+ROSA_DOCS_BASE : string
}
installLinks : OCP_DOCS_BASE changed from '.../openshift_container_platform/4.20/html' to '.../openshift_container_platform/latest/html'
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider extracting the repeated 'installing_on_google_cloud' path segment into its own constant to reduce duplication and ease future updates.
- Since you switched OCP docs to 'latest', review other doc bases (OSD/ROSA) for potential consistency or document why they remain versioned.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the repeated 'installing_on_google_cloud' path segment into its own constant to reduce duplication and ease future updates.
- Since you switched OCP docs to 'latest', review other doc bases (OSD/ROSA) for potential consistency or document why they remain versioned.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
revert replacement of `installing_on_google_cloud` with `installing_on_gcp`
# Conflicts: # src/common/installLinks.mjs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
zherman0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Code change looks fine. The test instructions say to run the check-links which I did. There are several issues still reported, but none have issue with using latest tg.
t-hendricks
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally. LGTM!
|
@eliranmal I tried to execute this PR a couple of times using the command run/check-links.mjs. Below are my results
The redirect URLs having been failing with 403 errors as shown below. Kindly take a look.
|
|
@shivanthi-amara - as noted in other PRs, these problems due to request errors are unrelated to the changes, and seem to only happen on your system. the only thing to really verify is that there's no regression; that the same number of successes/failures are reported before and after the change (provided that the request errors you're having are deterministic. matter of fact, you should not rely on them and either fix that or use another QE member's env for this verification) |
|
the request errors are apparently sporadic, and we were unable to determine their origin. after discussing it (@shivanthi-amara and myself), we agreed to try and hand the verification to another QE member, in hope they're able to run the check-links script without those sporadic errors. @shivanthi-amara - one lead i might have, is ECONNRESET errors are sometimes caused in the context of a bad VPN connection. maybe try to run the script without VPN? |
@jmekkatt Please have a try from your end as well to confirm. |
% ./run/check-links.mjs
Checking URLs...
Found 412 URLs to check
Testing redirect destinations...
URL CHECK RESULTS
Category Count
---------------------------------- ------
Total URLs skipped 0
Success 267
Redirects 141
Redirects errors 1
Client errors (4xx) 4
Server errors (5xx) 0
Request errors 0
---------------------------------- ------
Total URLs checked 412
SUCCESS: 267 URLs
All these URLs returned status code 200 (OK)
REDIRECTS: 141 URLs
[Status 301 - Moved Permanently (Resource at new location)] - 21 URLs
✓ 21 redirect URLs tested successfully
[Status 302 - Found (Resource temporarily at different URL)] - 83 URLs
✓ 82 redirect URLs tested successfully
X 1 redirect URLs had errors
[Original URL] https://console.aws.amazon.com/route53/v2/hostedzones
[Redirect URL] https://us-east-1.console.aws.amazon.com/route53/v2/hostedzones
[Redirect URL Test] 405
[Status 307 - Temporary Redirect (Resource temporarily moved)] - 37 URLs
✓ 37 redirect URLs tested successfully
CLIENT ERRORS (4xx): 4 URLs
[Status 405 - Method Not Allowed (Request method not supported)] - 4 URLs
[Original URL] https://console.aws.amazon.com/ec2/home#SecurityGroups
[Original URL] https://console.aws.amazon.com/rosa/home
[Original URL] https://console.aws.amazon.com/rosa/home#/get-started
[Original URL] https://issues.redhat.com/projects/OCPBUGS/issues
SERVER ERRORS (5xx): 0 URLs
No server errors (5xx) found
REQUEST ERRORS: 0 URLs
No request errors found
SKIPPED: 0 URLs
No URLs were skipped
--------------------------------------------------------------------------------
Total URLs checked: 412 URLs
--------------------------------------------------------------------------------
Note: Run with -v or --verbose to see URLs for successful requests and redirects
Run with -r or --redirects to see ONLY redirected URLs with targets
Run with -h or --help for more information |
|
@jmekkatt , it seems that this is the expected results, can you approve? or should @shivanthi-amara take it from here? |
|
@eliranmal I tried run links check several times. It isn't consistent with reporting same amount of the errors. Example: 3rd run: cc: @jmekkatt, @shivanthi-amara I will also check docs links manually as per linked test case to the Jira card |
|
@apecha - it shouldn't block approval cuz the links can be tested manually |






Summary
use "latest" in OCP doc-links, instead of hard-coding the version.
Additional information
just FYI: these were discovered during the work on this PR, and got moved to #204 for proper scoping:
installing_on_gcpgot replaced withinstalling_on_google_cloudJira
addresses OCMUI-3556
How to Test
run check-links, verify there's no regression in the output.
Review process
Please review and follow the PR process.
QE Reviewer