From 094ad53203e66c40491f6edbb7ace3cf5f83d81a Mon Sep 17 00:00:00 2001 From: Levi Waldron Date: Sat, 31 Oct 2026 21:31:57 -0400 Subject: [PATCH] Separate failure policies for R CMD check and BiocCheck Rename error_on to rcmdcheck_error_on and add bioccheck_error_on (error or never) so a known, unavoidable BiocCheck ERROR such as a Remotes: dependency not yet in Bioconductor no longer hides R CMD check results. No compatibility alias: the workflow is pre-release with seven known callers, and rename PRs follow on merge. Template, generator, usage docs and canary updated; ADR added as Accepted. Co-authored-by: Claude Code --- .github/workflows/bioccheck.yml | 21 +++++++--- .github/workflows/canary.yml | 7 ++-- README.md | 2 +- bioccheck_usage.md | 16 +++++++- ci.yml | 5 ++- .../0013-separate-bioccheck-failure-policy.md | 39 +++++++++++++++++++ docs/adr/README.md | 1 + docs/index.html | 30 ++++++++++---- technical_notes.md | 2 +- 9 files changed, 102 insertions(+), 21 deletions(-) create mode 100644 docs/adr/0013-separate-bioccheck-failure-policy.md diff --git a/.github/workflows/bioccheck.yml b/.github/workflows/bioccheck.yml index 6afe905..7791bae 100644 --- a/.github/workflows/bioccheck.yml +++ b/.github/workflows/bioccheck.yml @@ -18,11 +18,16 @@ on: required: false type: boolean default: false - error_on: - description: 'Error policy for rcmdcheck (never, note, warning, error)' + rcmdcheck_error_on: + description: 'R CMD check failure policy: fail the job on this severity or worse (never, note, warning, error)' required: false type: string default: 'warning' + bioccheck_error_on: + description: 'BiocCheck failure policy: error fails the job on any BiocCheck ERROR; never only reports' + required: false + type: string + default: 'error' bioc_version: description: 'Override Bioconductor version/container tag (e.g. devel, RELEASE_3_20). Auto-derived if empty.' required: false @@ -145,23 +150,29 @@ jobs: - name: Run R CMD check env: - RCMDCHECK_ERROR_ON: ${{ inputs.error_on }} + RCMDCHECK_ERROR_ON: ${{ inputs.rcmdcheck_error_on }} shell: Rscript {0} run: | allowed <- c("never", "note", "warning", "error") error_on <- Sys.getenv("RCMDCHECK_ERROR_ON", unset = "warning") - if (!error_on %in% allowed) stop("Invalid error_on value: '", error_on, "'. Must be one of: ", paste(allowed, collapse = ", ")) + if (!error_on %in% allowed) stop("Invalid rcmdcheck_error_on value: '", error_on, "'. Must be one of: ", paste(allowed, collapse = ", ")) res <- rcmdcheck::rcmdcheck(".", args = "--no-manual", error_on = error_on, check_dir = "check") print(res) - name: Run BiocCheck + env: + BIOCCHECK_ERROR_ON: ${{ inputs.bioccheck_error_on }} shell: Rscript {0} run: | + allowed <- c("error", "never") + error_on <- Sys.getenv("BIOCCHECK_ERROR_ON", unset = "error") + if (!error_on %in% allowed) stop("Invalid bioccheck_error_on value: '", error_on, "'. Must be one of: ", paste(allowed, collapse = ", ")) tarballs <- dir("check", pattern = "\\.tar\\.gz$", full.names = TRUE) if (length(tarballs) != 1) stop("Expected exactly one package tarball in check/, found ", length(tarballs)) + # BiocCheck exits non-zero only on ERRORs; WARNINGs and NOTEs never fail the job. BiocCheck::BiocCheck( tarballs, - `quit-with-status` = TRUE, `no-check-bioc-help` = TRUE + `quit-with-status` = identical(error_on, "error"), `no-check-bioc-help` = TRUE ) # Coverage is computed once and used by whichever of the Codecov upload diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index b27fe60..1dbe89f 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -2,8 +2,9 @@ name: Canary Check # Runs on a weekly schedule and on-demand to verify that bioccheck.yml # executes correctly end-to-end against the Bioconductor container tag derived from the branch (main => devel). -# Uses error_on: "never" so expected BiocCheck notes from this stub package -# (no vignettes, no NEWS, etc.) do not cause a false failure. +# Uses rcmdcheck_error_on: "never" so expected R CMD check findings from this stub package +# (non-standard top-level files such as ci.yml and docs/) do not cause a false failure. +# bioccheck_error_on keeps its default so the fatal BiocCheck path is exercised. # Enables cyclocomp so the conditional install and artifact upload are exercised. on: @@ -16,5 +17,5 @@ jobs: canary: uses: ./.github/workflows/bioccheck.yml with: - error_on: 'never' + rcmdcheck_error_on: 'never' enable_cyclocomp: true diff --git a/README.md b/README.md index 1a46a1c..8e74ef0 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ To show the check status at the top of your package's `README.md`, add this badg [![R CMD check](https://github.com///actions/workflows/ci.yml/badge.svg)](https://github.com///actions/workflows/ci.yml) ``` -To customize options (add Codecov, pkgdown, cyclocomp, or change check sensitivity from the default "warning") use the [Bioconductor GitHub Actions Workflow Generator](https://bioconductor.github.io/workflows/) and [add more status badges](bioccheck_usage.md#adding-status-badges-to-readmemd). +To customize options (add Codecov, pkgdown, cyclocomp, or change the failure policies from their defaults of "warning" for R CMD check and "error" for BiocCheck) use the [Bioconductor GitHub Actions Workflow Generator](https://bioconductor.github.io/workflows/) and [add more status badges](bioccheck_usage.md#adding-status-badges-to-readmemd). ## Multi-platform Testing using `r-universe-org/workflows` diff --git a/bioccheck_usage.md b/bioccheck_usage.md index 6784683..1fe8e62 100644 --- a/bioccheck_usage.md +++ b/bioccheck_usage.md @@ -19,7 +19,18 @@ R package dependencies are cached between runs, keyed on the package's dependenc To just run `R CMD check` and `BiocCheck` on commits and PRs, you can copy the [ci.yml](ci.yml) template into `.github/workflows/ci.yml` and no additional configuration is needed. -To add additional features like `COVR` and `pkgdown`, or change error sensitivity (`error/warning/note/never`), you can edit the above file, or use our interactive [Workflow Generator](https://bioconductor.github.io/workflows/). +To add additional features like `COVR` and `pkgdown`, or change error sensitivity, you can edit the above file, or use our interactive [Workflow Generator](https://bioconductor.github.io/workflows/). + +### Failure Policies + +`R CMD check` and `BiocCheck` have separate failure policies, because they measure different things. `R CMD check` is about whether the package is correct and installable; `BiocCheck` is about whether it meets Bioconductor submission guidelines, which the Bioconductor build system does not itself enforce after acceptance. + +* `rcmdcheck_error_on` (default `warning`): fail the job when `R CMD check` reports this severity or worse. One of `never`, `note`, `warning`, `error`. +* `bioccheck_error_on` (default `error`): `error` fails the job on any `BiocCheck` ERROR; `never` prints the `BiocCheck` report but does not fail the job. `BiocCheck` WARNINGs and NOTEs never fail the job under either setting. + +Set `bioccheck_error_on: 'never'` when a `BiocCheck` ERROR is known and unavoidable for now, so that `R CMD check` failures are still visible. The common case is a package under development that depends on a package not yet in Bioconductor or CRAN: `BiocCheck` raises an ERROR for any `Remotes:` or `Additional_repositories:` field in `DESCRIPTION`, and there is no way to silence that one check. Remove the override once the dependency is available from Bioconductor or CRAN. + +The `rcmdcheck_error_on` input was named `error_on` before 2026-11; callers must rename it. ### Repository Configuration for Optional Features @@ -104,7 +115,8 @@ Here's a full set of badges for the top of your package's `README.md`. Replace ` | `enable_pkgdown` | `boolean` | `false` | Enable building and deploying `pkgdown` site on release branch pushes. | | `enable_cyclocomp` | `boolean` | `false` | Enable cyclomatic complexity analysis via cyclocomp. | | `enable_coverage_report` | `boolean` | `false` | Compute test coverage with covr and save an HTML report as the `coverage-report` artifact, without Codecov. | -| `error_on` | `string` | `"warning"` | Error policy for `rcmdcheck` (`"never"`, `"note"`, `"warning"`, `"error"`). | +| `rcmdcheck_error_on` | `string` | `"warning"` | `R CMD check` failure policy (`"never"`, `"note"`, `"warning"`, `"error"`). | +| `bioccheck_error_on` | `string` | `"error"` | `BiocCheck` failure policy (`"error"`, `"never"`). | | `bioc_version` | `string` | `""` | Optional override for container tag (e.g. `devel`, `RELEASE_3_20`). | | `secrets.CODECOV_TOKEN` | secret | `""` | Optional Codecov token (repository or organization global upload token). Uploads coverage to Codecov when set. | diff --git a/ci.yml b/ci.yml index 6690203..df21ac7 100644 --- a/ci.yml +++ b/ci.yml @@ -14,8 +14,9 @@ jobs: # id-token: write uses: bioconductor/workflows/.github/workflows/bioccheck.yml@main with: - error_on: 'warning' + rcmdcheck_error_on: 'warning' + bioccheck_error_on: 'error' enable_pkgdown: false # Required only if you want test coverage via Codecov # secrets: - # CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} \ No newline at end of file + # CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/docs/adr/0013-separate-bioccheck-failure-policy.md b/docs/adr/0013-separate-bioccheck-failure-policy.md new file mode 100644 index 0000000..cb46a44 --- /dev/null +++ b/docs/adr/0013-separate-bioccheck-failure-policy.md @@ -0,0 +1,39 @@ +# 0013. Separate Failure Policies for `R CMD check` and `BiocCheck` + +- **Status:** Accepted (amends [ADR-0003](0003-configurable-rcmdcheck-severity.md)) +- **Date:** 2026-10-31 +- **Deciders:** Levi Waldron + +## Context and Problem Statement + +ADR 0003 made the `R CMD check` severity threshold configurable through the `error_on` input. `BiocCheck` had no equivalent: it always ran with `quit-with-status`, so any `BiocCheck` ERROR failed the job regardless of `error_on`. + +The two tools measure different things. `R CMD check` is about correctness and installability. `BiocCheck` is about conformance to Bioconductor submission guidelines, and the Bioconductor build system does not run it after a package is accepted. Some `BiocCheck` ERRORs are unavoidable for a period and cannot be silenced individually. The motivating case is a package under development that depends on a package not yet in Bioconductor or CRAN: `BiocCheck` raises an ERROR for any `Remotes:` or `Additional_repositories:` field in `DESCRIPTION`. The job is then permanently red, which hides `R CMD check` failures that the maintainer does care about. + +The workflow is pre-release, with seven known public callers, all pinned to `@main` and all setting `error_on` explicitly. + +## Decision + +Expose two inputs instead of one: + +- `rcmdcheck_error_on` (default `warning`), the input formerly named `error_on`, with the same four values. +- `bioccheck_error_on` (default `error`): `error` runs `BiocCheck` with `quit-with-status` so any ERROR fails the job; `never` prints the report and does not fail the job. Only two values exist because `BiocCheck` exits non-zero only on ERRORs; WARNINGs and NOTEs never fail the job. + +`error_on` is removed outright, with no alias. Callers that still pass it fail at workflow validation until they rename it, and one-line rename PRs are opened on all known callers the day this merges. + +The defaults preserve current behaviour for every caller once renamed. + +## Alternatives Considered + +- **Add `bioccheck_error_on` and leave `error_on` as is**: avoids a rename, but leaves one input with a generic name and one with a specific name. Rejected because the rename is cheap now and expensive after release. +- **A `skip_bioccheck` boolean**: simpler, but loses the `BiocCheck` report, which is still useful when only one known ERROR is being ignored. +- **A separate `BiocCheck` job with its own status**: most informative in the PR checks list, but roughly doubles runtime by pulling the container and installing dependencies twice, unless the tarball is passed as an artifact. Deferred until there is demand. +- **`continue-on-error` on the `BiocCheck` step, unconditionally**: hides real `BiocCheck` ERRORs from everyone. Rejected; the same effect is available per caller with `bioccheck_error_on: never`. +- **Keep `error_on` as a deprecated alias for a few weeks**: avoids a window where the known callers are red, but adds code, a deprecation message and a follow-up removal, for software that is pre-release and hardly used. Rejected; the red window is a day at most and every known caller is reachable. + +## Consequences + +- Maintainers can keep `R CMD check` strict while treating `BiocCheck` as advisory during development, and the job status means "R CMD check passed" in that configuration. The usage guide says so. +- Each new input must be mirrored in `ci.yml`, the Workflow Generator, and the usage table, per ADR 0007. This change does that. +- Any caller not in the known list that still passes `error_on` fails with a clear "unexpected input" error from GitHub Actions, and the usage guide names the rename. +- The canary workflow uses `rcmdcheck_error_on: never` and leaves `bioccheck_error_on` at its default, so it still exercises the fatal `BiocCheck` path. diff --git a/docs/adr/README.md b/docs/adr/README.md index bc0093b..31af938 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -16,3 +16,4 @@ This directory contains Architecture Decision Records (ADRs) for the `bioconduct - [0010. Install Optional Tools Only When Used](0010-conditional-tool-installation.md) - [0011. Upload Coverage with the Codecov Action](0011-codecov-action-upload.md) - [0012. Opt-in Coverage Report Without Codecov](0012-opt-in-coverage-report.md) +- [0013. Separate Failure Policies for `R CMD check` and `BiocCheck`](0013-separate-bioccheck-failure-policy.md) diff --git a/docs/index.html b/docs/index.html index 4020266..0798dc7 100644 --- a/docs/index.html +++ b/docs/index.html @@ -331,11 +331,11 @@

Configuration

- Error Policy (R CMD check) + Failure Policy (R CMD check) Determines which R CMD check results will cause the workflow to fail.
- @@ -343,6 +343,19 @@

Configuration

+ +
+
+ Failure Policy (BiocCheck) + BiocCheck warnings and notes never fail the workflow. Use "never" when a BiocCheck error is known and unavoidable for now, such as a Remotes: dependency not yet in Bioconductor. See usage guide. +
+
+ +
+
@@ -367,12 +380,13 @@

Generated YAML

const elCodecov = document.getElementById('opt-codecov'); const elCyclocomp = document.getElementById('opt-cyclocomp'); const elCoverageReport = document.getElementById('opt-coverage-report'); - const elErrorOn = document.getElementById('opt-error-on'); + const elRcmdcheckErrorOn = document.getElementById('opt-rcmdcheck-error-on'); + const elBiocCheckErrorOn = document.getElementById('opt-bioccheck-error-on'); const elOutput = document.getElementById('yaml-output'); const btnCopy = document.getElementById('copy-btn'); // Listeners - [elPkgdown, elCodecov, elCyclocomp, elCoverageReport, elErrorOn].forEach(el => { + [elPkgdown, elCodecov, elCyclocomp, elCoverageReport, elRcmdcheckErrorOn, elBiocCheckErrorOn].forEach(el => { el.addEventListener('change', generateYaml); }); @@ -381,7 +395,8 @@

Generated YAML

const hasCodecov = elCodecov.checked; const hasCyclocomp = elCyclocomp.checked; const hasCoverageReport = elCoverageReport.checked; - const errorOn = elErrorOn.value; + const rcmdcheckErrorOn = elRcmdcheckErrorOn.value; + const bioccheckErrorOn = elBiocCheckErrorOn.value; // Build the JavaScript Object Model const workflowObj = { @@ -407,8 +422,9 @@

Generated YAML

workflowObj.jobs.bioccheck.with.enable_pkgdown = true; } - // Add error_on parameter - workflowObj.jobs.bioccheck.with.error_on = errorOn; + // Add failure policy parameters + workflowObj.jobs.bioccheck.with.rcmdcheck_error_on = rcmdcheckErrorOn; + workflowObj.jobs.bioccheck.with.bioccheck_error_on = bioccheckErrorOn; // Add cyclocomp parameter if enabled if (hasCyclocomp) { diff --git a/technical_notes.md b/technical_notes.md index 3bd7be1..567e0ca 100644 --- a/technical_notes.md +++ b/technical_notes.md @@ -19,7 +19,7 @@ This repository uses `raven-actions/actionlint` to validate the GitHub Actions w ### [`canary.yml`](.github/workflows/canary.yml) The canary workflow runs on a weekly schedule (and on-demand) to verify that `bioccheck.yml` executes correctly end-to-end against the Bioconductor container tag derived from the main branch. -It uses `error_on: 'never'` so that expected BiocCheck notes from the internal stub package do not cause a false failure. +It uses `rcmdcheck_error_on: 'never'` so that expected `R CMD check` findings from the internal stub package (non-standard top-level files such as `ci.yml` and `docs/`) do not cause a false failure. `bioccheck_error_on` keeps its default (`error`), so a BiocCheck ERROR in the stub package still fails the canary. This repo contains a [DESCRIPTION](DESCRIPTION) file and [basic vignette](vignettes/canary.Rmd) to make it a valid R package that can be checked without error (with enough disabling flags invoked).