Skip to content

docs: branch the rjags install instructions by platform (closes #309) - #313

Merged
d-morrison merged 2 commits into
mainfrom
claude/gia-serodynamics-daytb-1lvmen
Sep 2, 2026
Merged

docs: branch the rjags install instructions by platform (closes #309)#313
d-morrison merged 2 commits into
mainfrom
claude/gia-serodynamics-daytb-1lvmen

Conversation

@d-morrison

@d-morrison d-morrison commented Sep 1, 2026

Copy link
Copy Markdown
Member

Closes #309.

What

.github/copilot-instructions.md told contributors to install rjags with type = "source" on every platform:

install.packages("rjags", repos = "https://cloud.r-project.org", type = "source")

Both .github/workflows/R-CMD-check.yaml and .github/workflows/test-coverage.yaml have branched on .Platform$OS.type since #308 — a source build on macOS/Linux, the binary on Windows:

if (identical(.Platform$OS.type, "windows")) {
  if (!requireNamespace("rjags", quietly = TRUE)) {
    install.packages("rjags", repos = "https://cloud.r-project.org", verbose = TRUE)
  }
} else {
  install.packages("rjags", repos = "https://cloud.r-project.org",
                   type = "source", verbose = TRUE)
}

So the document contradicted the CI it describes. This splits the snippet to match, and adds a line noting that the Docker snippet earlier in the file stays unconditional because that container is Linux.

Verified against both workflow files on main at 5be9001; the branching is byte-identical in each.

Scope

Documentation only. No workflow, R code, or test changes.

DESCRIPTION is bumped to 0.1.0.9020 (re-bumped in 7e671a6 after #317 merged with 0.1.0.9019) and a NEWS entry added, as version-check and Check Changelog Action require on every PR.

Merge order

main is merged in as of 7e671a6, so this PR carries no conflict with it. The remaining open PRs that also bump DESCRIPTION (#284, #298, #310, #311) will re-bump after this one lands, per the structural collision tracked in #312.

No file overlap with any other open PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_017trXKZFoCYC4kP7NKNnN6F

.github/copilot-instructions.md told contributors to install rjags with
type = "source" on every platform. Both R-CMD-check.yaml and
test-coverage.yaml have branched on .Platform$OS.type since #308, using a
source build on macOS/Linux and the binary on Windows, so the document
contradicted the CI it describes.

Split the snippet to match, and note that the Docker snippet earlier in the
file stays unconditional because that container is Linux.

Closes #309

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017trXKZFoCYC4kP7NKNnN6F

Copy link
Copy Markdown
Member Author

/review


Generated by Claude Code

@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

👀 Claude Review Dispatchedrun reviewing PR #313 at e8a8ef9.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


This is a small, accurate documentation fix. Everything checks out.

Review: PR #313 — docs: branch the rjags install instructions by platform (closes #309)

Summary

This PR fixes .github/copilot-instructions.md, which previously told contributors to install rjags with type = "source" on every platform. That contradicted CI: I verified both .github/workflows/R-CMD-check.yaml:69-88 and .github/workflows/test-coverage.yaml:52-71 branch on .Platform$OS.type == "windows" — installing the binary on Windows and building from source (type = "source") everywhere else — exactly as the PR describes. The new prose in .github/copilot-instructions.md:258-277 correctly reflects this branching and adds a clarifying note that the earlier Docker snippet (.github/copilot-instructions.md:246) stays unconditional because that container is Linux-only, which is accurate and not misleading. DESCRIPTION and NEWS.md are bumped/updated per repo convention (version-check / Check Changelog Action), consistent with what the lab manual's code-repositories conventions expect for every PR.

Findings

No bugs, hallucinations, or CLAUDE.md/lab-manual violations found:

  • No fabricated APIs, functions, or URLs — the cited workflow files and line behavior were verified directly against the checked-out repo and match.
  • Documentation-only change; no code touched, so the idiomatic-R-code review checklist (data-masking, joins, branching-on-columns, etc.) doesn't apply here.
  • Markdown renders correctly; no raw HTML, and code fences use fenced ```r blocks consistent with the rest of the file.
  • The version-bump collision called out in the PR description (shared 0.1.0.9019 with other open PRs) is a known, pre-existing structural issue tracked separately in ci: version-check forces every PR to claim the same dev version, so concurrent PRs collide on DESCRIPTION #312 — not something to block this PR on.

Nothing to flag.

Verdict

Ready for merge

Structured Review Data (JSON)
{
  "schema_version": "1.0",
  "reviewer": "claude",
  "commit_sha": "e8a8ef9721a726c31854357a2b081c588f2bcffe",
  "verdict": "CLEAN",
  "findings": []
}

Reviewed commit: e8a8ef9

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

💰 Cost: $0.1962 (review) — run

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

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

Resolve the NEWS.md conflict by keeping both Internal bullets, and
re-bump DESCRIPTION past the 0.1.0.9019 that #317 took.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017trXKZFoCYC4kP7NKNnN6F

Copy link
Copy Markdown
Member Author

The user asked for this PR to be merged once it is ready, after #317. #317 merged as 1118ac7 and took 0.1.0.9019, so 7e671a6 merges main into this branch, keeps both ## Internal NEWS bullets, and re-bumps DESCRIPTION to 0.1.0.9020. No other file changed. Merging once CI is green on 7e671a6 and the review is clean.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

Copy link
Copy Markdown
Member Author

CI on 7e671a6: windows-latest (release) failed at the rjags load step with Failed to locate any version of JAGS version 4: the rjags 4-17 binary installed, but the JAGS runtime the earlier installer step is meant to place was not found, so the JAGS download or installer did not complete on that runner. Not this PR's: the diff is .github/copilot-instructions.md prose, DESCRIPTION, and NEWS.md, with no workflow change, and the same job passed on this branch's previous head e8a8ef9 and on main's current tip 1118ac7 (#317's head, 21:38 PDT run). Every other job on 7e671a6 is green so far. Re-running the failed job once; if it fails again the JAGS install step needs a look on its own issue rather than here.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

@d-morrison
d-morrison merged commit ff3c79c into main Sep 2, 2026
18 of 19 checks passed
@d-morrison
d-morrison deleted the claude/gia-serodynamics-daytb-1lvmen branch September 2, 2026 05:27
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.

docs: update copilot-instructions.md JAGS installation notes for OS branching

2 participants