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

Move rmarkdown::render action to happen in a temp directory #330

Merged
merged 3 commits into from
May 18, 2024

Conversation

bburns632
Copy link
Collaborator

This resolves #329 (thanks @jcarbaut for the catch!)

Better detailed in the bug issue above, the issue is that when we called rmarkdown::render to render the report from an Rmd file within the installed package directory, it creates interim files within the installed package directory. This is technically a CRAN violation. It is rectified in line with rmarkdown's recommendation for this issue which is to move all Rmd files to a temp directory and then call rmarkdown::render there.

Why didn't we catch this before?
My money is on:

  • The authors use more forgiving directories for installation, so missed the opportunity to catch this issue here.
  • Our test process involves installing pkgnet as well as all test packages in a temporary directory for testing. Therefore, these modifications of the installed directory were happening within the temp directory, leaving the original directory untouched.
  • The interim files are created and deleted within the run, so a cursory before and after look at file structure won't necessarily catch this.

Why didn't CRAN catch this before?
We have passed CRAN checks for years. My guess is that that our unique process of installing within a temp directory for testing is what allowed this error to go undetected.

For our unit tests, we now leverage file.info() to compare last modification date for the package directory (installed within a temporary directory) before and after testing. A change in this date can catch this error.

@bburns632 bburns632 force-pushed the bugfix_root_render branch from 8238989 to 85df18f Compare May 18, 2024 16:23
@bburns632 bburns632 merged commit 994723d into main May 18, 2024
6 checks passed
@bburns632 bburns632 deleted the bugfix_root_render branch May 18, 2024 16:33
@jameslamb
Copy link
Collaborator

Great description, thanks @bburns632

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.

Permission denied
2 participants