Skip to content

Add a bullet for rhub::rhub_check(platforms = 'rchk') if a package has a src/ directory#2209

Open
DavisVaughan wants to merge 1 commit intomainfrom
feature/rhub-rchk
Open

Add a bullet for rhub::rhub_check(platforms = 'rchk') if a package has a src/ directory#2209
DavisVaughan wants to merge 1 commit intomainfrom
feature/rhub-rchk

Conversation

@DavisVaughan
Copy link
Member

I am having to do a patch vctrs release due to some missed rchk results
r-lib/vctrs#2139

This feels avoidable, what if we add it to our release bullets if we see a src/ directory in the package?

@DavisVaughan
Copy link
Member Author

DavisVaughan commented Jan 22, 2026

Hmm it seems like we removed all of rhub in #1773

I was hesitant to remove the rchk and sanitizers builds at that point in time, and I have now been bitten by this, so I'd advocate for bringing this one back. Looks like we were using the same src/ condition there as well

The big difference with rchk is that it only gets caught after your package has been accepted on CRAN. So catching this early is worth it.

@DavisVaughan DavisVaughan requested a review from jennybc January 22, 2026 13:58
@jennybc
Copy link
Member

jennybc commented Jan 27, 2026

I certainly don't have any strong objection to a bullet re: rchk.

But for discussion's sake: lately I've been adding a GHA workflow for rhub in packages with compiled code. Then I can do a variety of ad hoc checks using workflow dispatch, e.g. rchk or checking with sanitizers, etc.

Examples:

https://github.com/tidyverse/vroom/blob/main/.github/workflows/rhub.yaml

https://github.com/tidyverse/readxl/blob/main/.github/workflows/rhub.yaml

I tend to fiddle with this line depending on what my main concerns are for that release:

https://github.com/tidyverse/readxl/blob/e090951f2b8065e1b82d8dfa0f071ae9e7c9c8c1/.github/workflows/rhub.yaml#L30

I think I've gravitated towards this because if I have some rchk or sanitizer problem, I'm usually working through it in a PR and I like to keep running whatever rhub check is helping me.

Here's me running rchk on vroom via workflow dispatch and via rhub::rhub_check(platforms = 'rchk') (the proposed bullet). Same result.

Screenshot 2026-01-27 at 3 17 53 PM

Do you have any thoughts re: whether we prefer the ad hoc run vs. workflow dispatch? 🤔

@DavisVaughan
Copy link
Member Author

These feel a bit orthogonal in purpose to me, I think.

vctrs also has an rhub.yaml because, of course, it is needed to run rhub::rhub_check(platforms = 'rchk')
https://github.com/r-lib/vctrs/blob/main/.github/workflows/rhub.yaml

So to me there are two distinct cases:

  • You're actively working on an issue that you know has rchk or sanitizer implications. In that case, you either run rhub::rhub_check(platforms = 'rchk') locally while on the branch of interest, or you go to the Actions page for that workflow and trigger it yourself. Either of these are pretty manual, either feels reasonable to me.

  • You're about to release a package with compiled code. It is quite important not to forget to run rhub::rhub_check(platforms = 'rchk').

The 2nd bullet is all I'm trying to target with this PR.


@jennybc I believe you actually do a variant of the 1st bullet. Something you didn't mention is that your rhub.yaml fires on pull_request:. That's not the standard workflow. It's typically only via some manual invocation.

So you're getting these checks on every PR, IIUC. And you just take an active role in adjusting exactly which checks run on that PR depending on the need.

I think that's totally fine and reasonable for sensitive things like vroom, and it's very proactive since it means you probably can't merge anything to main that might have an rchk issue.

But I'm not sure we need to be quite so defensive in all of our packages. I think the rchk runner takes some time, so I don't want it to slow us down in general. Mainly I just want to be sure that we don't release a package to CRAN with an rchk issue, and the release bullet feels good enough to me for that.

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.

2 participants