-
Notifications
You must be signed in to change notification settings - Fork 26
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
Ignore renv files and folders during checks #107
Conversation
Hold on before consider a merge. I want to add an option to toggle this functionality on/off, as we probably want to detect (and complain) about "renv" files being pushed to <git.bioconductor.org>, while allowing users to disable the functionality on their local machine (when it crashes BiocCheck as for me). I'll ping back when it's fully ready. The current version is functional in that it always ignores "renv" files. |
It looks ready to me now. Happy to hear feedback. Example usages and outputs:
Note that
|
I'm not really in favor of this pull request as it stands. A few things : I'm curious as to The renv files and functionality should be The |
Hi @lshep I agree that this PR should be considered a draft, and I'm happy to edit as needed, following a discussion of what standard BiocCheck should ultimately enforce. Starting with this point:
The coding practices checks are given the package root directory, which lets some checks parse all subdirectories. Other checks are explicitly checking specific subdirectories (e.g. While it is technically possible to run separate commands to search for R script in an explicit list of target locations (
From a more technical perspective, BiocCheck is choking because of some files in the Then,
Correct.
See https://rstudio.github.io/renv/articles/renv.html Next,
Not having the subdirectory checked is what this PR is about, at least on the developer's computer.
I tried to keep the message reasonably short to fit in the BiocCheck report, but I'm open to rephrasing to make the ERROR clearer or more explicit. So the ERROR message could be updated to something like:
Next,
Absolutely agreed. Finally,
Again, the Let me know what you think! |
Using version from commit f653c9f Full check (intended for the BBS):
Check ignoring "renv" files (intended for developer's computer):
|
A good place for more extended instructions is in the vignette, with the error message saying to look there for a more complete explanation. |
The only coding practice check that doesn't check only R man or vignette seems to be the checkSystemCall which for consistency probably should be updated to the limited set of directories. If the checkSystemCall was updated for that it seems your issues would be resolved? |
See https://github.com/Bioconductor/BiocCheck/blob/master/R/util.R#L721 My PR ignores all files under the "renv" subdirectory, to avoid sourcing code that is not part of the package. Again, this is all only to allow BiocCheck to run on the developer's computer, where "renv" files can be expected, using the I agree that "renv" files should never be git tracked, (neither GitHub nor Bioconductor git), and that the vignette should educate developers about ignoring those files from the start, or untracking those files if they were added by accident. |
I take it that's a "no" then :) |
I also wonder what happened. A few words would have been nice. |
Hi Kevin! @kevinrue I was doing clean up of branches and deleted the Regarding the aims of the PR, I'm open to implementing checking of stray From what I gather, the issue is more related to |
name: Ignore renv files and folders during checks
about: Submit a pull request that resolves an BiocCheck issue #106
title: "[PR] Your pull request"
labels: ''
assignees: ''
If you are submitting a pull request to
BiocCheck
please follow the instructions outlined in this presentation. This presentation includes steps for forking, creating a branch for you to work on, and useful related information.Prior to sending the pull request, please verify that
R CMD build
andR CMD check
run without warnings or errors on the latest Bioconductor-devel (currently in May 2020 that would be Bioconductor 3.12) and complete the following steps:The easiest way to obtain a working environment with Bioconductor-devel in your computer is to use the Bioconductor devel docker image as described in detail in the Bioconductor website.
For more questions, please get in touch with the Bioconductor core team through the Bioconductor Slack workspace.