Skip to content

Point PR-preview GitHub links at the PR (closes #136)#137

Open
d-morrison wants to merge 4 commits into
mainfrom
claude/issue-136-pr-preview-links
Open

Point PR-preview GitHub links at the PR (closes #136)#137
d-morrison wants to merge 4 commits into
mainfrom
claude/issue-136-pr-preview-links

Conversation

@d-morrison
Copy link
Copy Markdown
Owner

Closes #136.

Summary

  • docs.yaml: on PR builds, run yq against altdoc/quarto_website.yml before altdoc::render_docs() to rewrite three keys:
    • website.repo-urlhttps://github.com/<head_repo_full_name>
    • website.repo-branch<head_ref> (so quarto's "Edit this page" / "View source" land on the right file in the right branch)
    • website.navbar.right[icon == "github"].href<pull_request.html_url> (so the navbar GitHub icon opens the PR conversation page)
  • altdoc/quarto_website.yml: add a short comment next to the three keys so it's not a surprise that they get mutated in CI.
  • Non-PR builds (push to main, releases, workflow_dispatch) leave the YAML alone.

yq is preinstalled on ubuntu-latest, so no extra setup step needed.

Test plan

  • On this PR's preview, the navbar GitHub icon should open https://github.com/d-morrison/rpt/pull/<this PR#>.
  • On any page, "Edit this page" should resolve to https://github.com/<head_repo>/edit/<head_ref>/..., not main.
  • After this PR merges, a build of main should produce a site whose navbar GitHub icon points at the repo root ($ALTDOC_PACKAGE_URL_GITHUB substituted by altdoc), confirming non-PR builds were left untouched.

🤖 Generated with Claude Code

On PR-preview builds, the navbar GitHub icon now opens the PR conversation
page and quarto's "Edit this page" / "View source" actions land on the PR
head branch instead of UCD-SERG/rpt main. Non-PR builds are untouched.

docs.yaml gains a `yq` step before `altdoc::render_docs()` that rewrites
three keys in altdoc/quarto_website.yml: `website.repo-url`,
`website.repo-branch`, and the `website.navbar.right[icon == github].href`.
Sourced from `github.event.pull_request.head.repo.full_name`, `head.ref`,
and `html_url`. yq is preinstalled on ubuntu-latest.

A comment in altdoc/quarto_website.yml flags that those three keys are
mutated in PR builds, so the in-place rewrite isn't a surprise to anyone
reading the YAML cold.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 19, 2026 03:34
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the documentation workflow so PR preview builds point Quarto repository links at the pull request’s head repository/branch and the navbar GitHub icon at the PR conversation.

Changes:

  • Adds a PR-only workflow step that rewrites Quarto repo link settings before rendering docs.
  • Documents the CI mutation behavior in altdoc/quarto_website.yml.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/docs.yaml Adds the PR-preview yq rewrite step before altdoc::render_docs().
altdoc/quarto_website.yml Adds a comment explaining CI-time rewriting of GitHub link-related settings.

Comment thread altdoc/quarto_website.yml Outdated
Comment on lines +18 to +21
# `repo-url`, `repo-branch`, and the navbar github icon's `href` below are
# rewritten in-place by .github/workflows/docs.yaml on PR-preview builds, so
# "Edit this page" / "View source" / the navbar GitHub link point at the PR
# branch and the PR conversation page instead of the main repo.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@d-morrison
Copy link
Copy Markdown
Owner Author

@claude apply changes based on the comments in this thread then review and then revise

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 19, 2026

Claude finished @d-morrison's task in 1m 32s —— View job


Applied review feedback + PR review

Tasks completed:

  • Read relevant files for context
  • Apply Copilot review feedback to altdoc/quarto_website.yml
  • Review the full changes
  • Commit and push revisions

Change applied

The comment in altdoc/quarto_website.yml previously said all three keys are "rewritten in-place", which was misleading because repo-branch doesn't exist in that file — the workflow inserts it dynamically. Updated to:

# On PR-preview builds, .github/workflows/docs.yaml rewrites `repo-url` and
# the navbar GitHub icon's `href` below, and inserts `repo-branch` (absent
# here; added dynamically), so "Edit this page" / "View source" / the navbar
# GitHub link point at the PR branch and PR conversation page, not main.

Code review

.github/workflows/docs.yaml — new step (lines 115–124)

Correctness

  • if: github.event_name == 'pull_request' correctly gates the step so non-PR builds leave the YAML untouched.
  • Values are passed through env: vars and read with strenv(), which avoids shell injection from branch names or PR URLs containing special characters.
  • yq is preinstalled on ubuntu-latest — no extra setup needed.
  • Step is placed before Build site (the altdoc::render_docs() call), so the mutated YAML is picked up at render time. Ordering is correct.
  • repo-url and the navbar href are rewritten (keys already exist); repo-branch is inserted as a new key. All three yq calls work as intended.

Robustness

  • The select(.icon == "github") filter silently matches nothing if no item in navbar.right[] has icon: github, so there's no risk of the step erroring if the YAML structure changes. Worth noting in docs but not a bug.
  • Fork PRs: github.event.pull_request.head.repo.full_name correctly resolves to the forker's repo, so "Edit this page" links will point to the right fork and branch for cross-repo PRs. This is intentional and correct.

Minor observations (no action required)

  • The three yq -i calls could be collapsed into one compound yq expression for a slight speedup, but the current three-liner is easier to read and maintain — fine as-is.
  • repo-url on the main branch still points to https://github.com/UCD-SERG/rpt, which diverges from the current repo owner. That's a pre-existing issue unrelated to this PR.

altdoc/quarto_website.yml — comment (lines 18–21)

After today's fix: clearly distinguishes which keys are rewritten vs. inserted. ✅


Summary

The implementation is correct. The only substantive change needed was the comment clarification, which has been committed and pushed to claude/issue-136-pr-preview-links.
·

github-actions Bot and others added 2 commits May 19, 2026 07:36
The previous comment said all three keys are "rewritten in-place", but
`repo-branch` does not exist in this file — the workflow inserts it
dynamically on PR-preview builds.

Co-authored-by: Douglas Ezra Morrison <d-morrison@users.noreply.github.com>
If the navbar selector (`.website.navbar.right[] | select(.icon == "github")`)
ever fails to match — e.g. because someone renames the entry or the version
dropdown script later widens its scope — the yq edit silently no-ops and the
preview site keeps pointing at main. Read the three values back after the
rewrites and fail the workflow if any of them isn't what we expected, so the
regression surfaces immediately in CI rather than as a confusing user
experience on the preview.

Also added a comment pinning the step ordering (must run after the version
dropdown generator) to head off subtle future breakage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 19, 2026 07:46
Two issues from round 2:
- `yq -i '.website.X = ...'` will *create* X if .website itself has been
  renamed or restructured, so the postcondition check would happily read
  back the value we just wrote into a key quarto ignores. Add a
  precondition pass with `yq -e` that fails the workflow if either
  `.website.repo-url` or the navbar `icon: github` entry is missing
  before we attempt to rewrite anything.
- Replaced the `[ A ] || [ B ] || [ C ]` line-continuation chain with a
  single `[[ A || B || C ]]` to remove the trailing-whitespace footgun.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@github-actions
Copy link
Copy Markdown
Contributor

PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://d-morrison.github.io/rpt/pr-preview/pr-137/

Built to branch gh-pages at 2026-05-19 07:51 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PR previews: GitHub links should point at PR conversation / head branch

2 participants