r-lib / devtools

Tools to make an R developer's life easier
https://devtools.r-lib.org
Other
2.37k stars 755 forks source link

fix documentation of default values for `on_error` parameter #2529

Closed jan-glx closed 8 months ago

jan-glx commented 11 months ago

Currently the documentation for devtools::check's on_error parameter reads as inherited from rcmdcheck::rcmdcheck:

Whether to throw an error on R CMD check failures. Note that the check is always completed (unless a timeout happens), and the error is only thrown after completion. If "never", then no errors are thrown. If "error", then only ERROR failures generate errors. If "warning", then WARNING failures generate errors as well. If "note", then any check failure generated an error. Its default can be modified with the RCMDCHECK_ERROR_ON environment variable. If that is not set, then "never" is used.

However, devtools already sets the default to "never" (for interactive session) or "warning" (for non-nteractive sessions) according to: https://github.com/r-lib/devtools/blob/2c642ab44f4fb9d4117f0e198dfcf42ad79e432a/R/check.R#L56 https://github.com/r-lib/devtools/blob/2c642ab44f4fb9d4117f0e198dfcf42ad79e432a/R/check.R#L72 https://github.com/r-lib/devtools/blob/2c642ab44f4fb9d4117f0e198dfcf42ad79e432a/R/check.R#L82-L85

This PR copies the description to of on_error to devtools::check and fixes the description of this default behavior.

jan-glx commented 11 months ago

A potentially cleaner alternative could be to set the default for on_error to if(interactive()) "never" else "warning" within the function's formals, while leaving the error checking (the match.arg call) to rcmdcheck::rcmdcheck.

jennybc commented 8 months ago

Sorry I just did a fix for this myself, because I didn't realize this PR was here. Next time if you refer to the associated issue using the Magic Syntax, it will create a link and make these things much easier for others to discover.

Thanks for the PR anyway!

https://code-review.tidyverse.org/author/submitting.html#sec-github-features

jan-glx commented 8 months ago

Of course! - did not realize there was an open issue about this... your solution referencing rcmdcheck::rcmdcheck documentation is better anyways!