Bioconductor / BiocCheck

http://bioconductor.org/packages/BiocCheck
8 stars 26 forks source link

Make it possible to enable/disable individual checks via env var R_BIOCCHECK_CHECK_nnn #79

Closed HenrikBengtsson closed 3 years ago

HenrikBengtsson commented 4 years ago

Similarly how R CMD check has _R_CHECK_nnn_ environment variables, make it possible to enable/disable individual checks via environment variables, e.g. R_BIOCCHECK_CHECK_CITATION=false.

mtmorgan commented 4 years ago

FWIW I'm a negative fan of environment variables (and of conditional evaluation!). How are we supposed to know what environment variables do what magic (or that the variable exists in the first place, or are set on the environment we are running under?)? And at some level, 'if it's good enough for CRAN, then it's good enough for ME' so let's just drop --as-cran and have a global standard for what good (n.b., not 'best') practices are.

HenrikBengtsson commented 4 years ago

Being able to disable checks is useful when checks are under construction, you know they produce false positives, etc. Being able to get exit code 0 from an R CMD check is useful when you run continuous integration systems.

How are you planning to test newly introduced checks without being able to turn them on and off easily? The only alternative I see is then to rollback a BiocCheck update that was too strict or gave too many false positives?

Regarding CRAN checks; the way I understand it is that several of the checks in --as-cran, but also others controllable by other _R_CHECK_nnn_, will make it into the default R CMD check. The cannot just enable them yet because that'll break loooots of packages. On approach CRAN is doing to tighten up things is to require strictly all OKs upon CRAN updates. This way they'll eventually got enough packages fixed and then they can enforce it on remaining package developers. The _R_CHECK_LENGTH_1_LOGIC2_ and _R_CHECK_LENGTH_1_CONDITION_ are good examples of this.

PS. I'm a person favoring R CMD check results with all OKs without any WARNINGs or NOTEs. Maybe that clarifies where I'm coming from.

hpages commented 4 years ago

In the context of using R CMD BiocCheck more systematically e.g. as part of the daily builds, I think the BiocCheck results would be informative only, at least for a while, and would not be taken into consideration for propagation or for sending automatic build failure notifications. Until BiocCheck is stable and trusted enough but this might take months if not years. In that context false positives, false negatives and other flaws should not be too much of a nuisance. It will actually help identify and fix them if they are included in the daily build/check report.

lshep commented 4 years ago

FYI: The top level checks in BiocCheck have flags to shut them off because it was requested by other labs that wanted shut off the Bioconductor support site checks. Instead of picking and choosing we made flags for each section.

HenrikBengtsson commented 3 years ago

Great. The added CLI options and function arguments covers my needs. Thxs