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

Lechee check on PR is checking whole portal, instead of just modified content #101

Open
georgik opened this issue Sep 3, 2024 · 3 comments
Assignees
Labels
Priority: Low Minor improvement, automation, or typos in content Status: In Progress Issue is being worked on Type: Bug Bug in developer-portal

Comments

@georgik
Copy link
Collaborator

georgik commented Sep 3, 2024

The check should be localized only to added content, because otherwise the merge might fail in case of malfunction of url in unrelated content.

@f-hollow
Copy link
Collaborator

#107 limits link checking by pr-check-links to the PR diffs only.

@f-hollow
Copy link
Collaborator

There were numerous false positives in recent PRs.

Analyzing false positives yielded the following results:

  • #149 and #167
    • Dump links from the commit after which the feature branch was created (as opposed to checking out current main and dumping the links)
  • #164
    • Mimicking the pr-link-check workflow locally yielded false positives differing from the ones reported by GitHub Actions.
    • Among the mentioned false positives, the link https://docs.google.com/forms/d/e/1FAIpQLSeqeP4L90wLu0om38q-wvxKYKI1_Y4Hf4T928NQI8LBW4mHhQ/viewform?embedded=true was present in .lycheeignore, yet it still triggered false positives, escaping the question mark ...\?embedded... solved the problem.
    • Mimicking the pr-link-check workflow outside of the company network didn't yield any false positives!
    • While mimicking false positives in this and other PRs, I eventually discovered that lychee dumps only part of links at times (some files are skipped?). lychee's Troubleshooting section suggests "breaking the check into multiple lychee runs with different exclusion/inclusion rules". It might help resolve such false positives.

Actions required:

  • Update the pr-link-check workflow to have the links dumped from the commit after which the feature branch was created
    git checkout -b <feature-branch>-main
    git reset --hard HEAD (discard all feature branch commits)
    dump all links using lychee

@f-hollow
Copy link
Collaborator

f-hollow commented Jan 22, 2025

Summary of work done

#107

This PR implements link checking in diffs. To be more precise, the links that exist in main's latest commit are added to .lycheeignore, then the links that exist in the contributor's feature branch are checked for validity.

#180

On a number of occasions, broken links from the main branch were reported.

It was decided to dump links from main only until the commit at which a feature branch was forked (base commit). That would provide more predictable results.

#366

It appeared that main branches in forks were sometimes not rebased for a while, and the feature branches was rebased on upstream/main directly. As a result, the base commit was determined incorrectly (latest but still outdated common commit on main and feature). For this reason, the broken links from the most recent commits on main seeped through.

This PR changes the branch against which the base commit is determined from hard-coded origin/main to the branch defined by the CI environment variables TARGET_REPO_URL and TARGET_BRANCH. These variables are set to the upstream/main branch.

Follow-up actions

The same links from old commits still occasionally pop up in different PRs. During work on #379, it became clear that even though files containing false positives are not changed in PRs directly, Hugo still updates them. More information can be found in #379 and further investigation is needed.

@f-hollow f-hollow added Status: In Progress Issue is being worked on Type: Bug Bug in developer-portal Priority: Low Minor improvement, automation, or typos in content labels Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Low Minor improvement, automation, or typos in content Status: In Progress Issue is being worked on Type: Bug Bug in developer-portal
Projects
None yet
Development

No branches or pull requests

2 participants