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

devtools::check(error_on="warning") doesn't error on warnings from documenting #2557

Open Shoeboxam opened 4 months ago

Shoeboxam commented 4 months ago

I'm surprised to see the devtools check command pass when configured to error on warnings, and then still see warnings in the build output.

For example, see the start of the check step here: https://github.com/opendp/opendp/actions/runs/7904579655/job/21575444021?pr=1253

══ Documenting ═════════════════════════════════════════════════════════════════ ℹ Updating opendp documentation ℹ Loading opendp Registered S3 method overwritten by 'opendp': method from print.hashtab utils ✖ mod.R:390: S3 method `to_str.default` needs @export or @exportS3method tag. ✖ mod.R:391: S3 method `to_str.hashtab` needs @export or @exportS3method tag. ══ Building ════════════════════════════════════════════════════════════════════ Setting env vars: • CFLAGS : -Wall -pedantic • CXXFLAGS : -Wall -pedantic • CXX11FLAGS: -Wall -pedantic • CXX14FLAGS: -Wall -pedantic • CXX17FLAGS: -Wall -pedantic • CXX20FLAGS: -Wall -pedantic ── R CMD build ───────────────────────────────────────────────────────────────── * checking for file ‘/home/runner/work/opendp/opendp/R/opendp/DESCRIPTION’ ... OK * preparing ‘opendp’: * checking DESCRIPTION meta-information ... OK * cleaning src * checking for LF line-endings in source and make files and shell scripts * checking for empty or unneeded directories Removed empty directory ‘opendp/.github’ Removed empty directory ‘opendp/tests/testthat/_snaps’ * building ‘opendp_0.9.2.tar.gz’ ══ Checking ════════════════════════════════════════════════════════════════════ Setting env vars: • _R_CHECK_CRAN_INCOMING_REMOTE_ : FALSE • _R_CHECK_CRAN_INCOMING_ : FALSE • _R_CHECK_FORCE_SUGGESTS_ : FALSE • _R_CHECK_PACKAGES_USED_IGNORE_UNUSED_IMPORTS_: FALSE • NOT_CRAN : true ── R CMD check ───────────────────────────────────────────────────────────────── * using log directory ‘/tmp/RtmptYflUU/file1ab4512e26db/opendp.Rcheck’ * using R version 4.3.2 (2023-10-31) * using platform: x86_64-pc-linux-gnu (64-bit) * R was compiled by gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0 GNU Fortran (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0 * running under: Ubuntu 22.04.3 LTS * using session charset: UTF-8 * using options ‘--no-manual --as-cran’ * checking for file ‘opendp/DESCRIPTION’ ... OK * this is package ‘opendp’ version ‘0.9.2’ * package encoding: UTF-8 * checking package namespace information ... OK * checking package dependencies ... OK * checking if this is a source package ... OK * checking if there is a namespace ... OK * checking for executable files ... OK * checking for hidden files and directories ... OK * checking for portable file names ... OK * checking for sufficient/correct file permissions ... OK * checking whether package ‘opendp’ can be installed ... OK * used C compiler: ‘gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0’ * checking installed package size ... NOTE installed size is 109.1Mb sub-directories of 1Mb or more: libs 108.4Mb * checking package directory ... OK * checking for future file timestamps ... OK * checking DESCRIPTION meta-information ... OK * checking top-level files ... NOTE Non-standard files/directories found at top level: ‘_pkgdown.yml’ ‘opendp.Rproj’ * checking for left-over files ... OK * checking index information ... OK * checking package subdirectories ... OK * checking R files for non-ASCII characters ... OK * checking R files for syntax errors ... OK * checking whether the package can be loaded ... OK * checking whether the package can be loaded with stated dependencies ... OK * checking whether the package can be unloaded cleanly ... OK * checking whether the namespace can be loaded with stated dependencies ... OK * checking whether the namespace can be unloaded cleanly ... OK * checking loading without being on the library search path ... OK * checking dependencies in R code ... OK * checking S3 generic/method consistency ... OK * checking replacement functions ... OK * checking foreign function calls ... OK * checking R code for possible problems ... OK * checking Rd files ... OK * checking Rd metadata ... OK * checking Rd line widths ... OK * checking Rd cross-references ... OK * checking for missing documentation entries ... OK * checking for code/documentation mismatches ... OK * checking Rd \usage sections ... OK * checking Rd contents ... OK * checking for unstated dependencies in examples ... OK * checking line endings in shell scripts ... OK * checking line endings in C/C++/Fortran sources/headers ... OK * checking line endings in Makefiles ... OK * checking compilation flags in Makevars ... NOTE Package has both ‘src/Makevars.in’ and ‘src/Makevars’. Installation with --no-configure' is unlikely to work. If you intended ‘src/Makevars’ to be used on Windows, rename it to ‘src/Makevars.win’ otherwise remove it. If ‘configure’ created ‘src/Makevars’, you need a ‘cleanup’ script. * checking for GNU extensions in Makefiles ... OK * checking for portable use of $(BLAS_LIBS) and $(LAPACK_LIBS) ... OK * checking use of PKG_*FLAGS in Makefiles ... OK * checking use of SHLIB_OPENMP_*FLAGS in Makefiles ... OK * checking pragmas in C/C++ headers and code ... OK * checking compilation flags used ... OK * checking compiled code ... OK * checking examples ... OK * checking for unstated dependencies in ‘tests’ ... OK * checking tests ... Running ‘testthat.R’ OK * checking for non-standard things in the check directory ... OK * checking for detritus in the temp directory ... OK * DONE Status: 3 NOTEs See ‘/tmp/RtmptYflUU/file1ab4512e26db/opendp.Rcheck/00check.log’ for details. ── R CMD check results ─────────────────────────────────────── opendp 0.9.2 ──── Duration: 25.9s ❯ checking installed package size ... NOTE installed size is 109.1Mb sub-directories of 1Mb or more: libs 108.4Mb ❯ checking top-level files ... NOTE Non-standard files/directories found at top level: ‘_pkgdown.yml’ ‘opendp.Rproj’ ❯ checking compilation flags in Makevars ... NOTE Package has both ‘src/Makevars.in’ and ‘src/Makevars’. Installation with --no-configure' is unlikely to work. If you intended ‘src/Makevars’ to be used on Windows, rename it to ‘src/Makevars.win’ otherwise remove it. If ‘configure’ created ‘src/Makevars’, you need a ‘cleanup’ script. 0 errors ✔ | 0 warnings ✔ | 3 notes ✖

Is it possible to configure devtools to include warnings from documenting?

Thanks!

jennybc commented 4 months ago

Which lines of the GHA log are you referring to?

I don't see any warnings, just NOTES:

0 errors ✔ | 0 warnings ✔ | 3 notes ✖

https://github.com/opendp/opendp/actions/runs/7904579655/job/21575444021?pr=1253#step:8:154

Shoeboxam commented 4 months ago

Hi Jenny, thanks for the response! This part here located at the top of the log:

Registered S3 method overwritten by 'opendp':
  method        from 
  print.hashtab utils
✖ mod.R:390: S3 method `to_str.default` needs @export or @exportS3method tag.
✖ mod.R:391: S3 method `to_str.hashtab` needs @export or @exportS3method tag.

I think these warnings somehow got missed?

The three notes seem to be for other, unrelated things.

jennybc commented 4 months ago

error_on has a very specific sphere of influence and it's narrower than what you seem to be expecting:

Whether to throw an error on R CMD check failures....

There has been other confusion about error_on (#2506) and, as a result, the documentation of devtools::check(error_on =) in the dev version of devtools is different (hopefully better).

https://devtools.r-lib.org/dev/reference/check.html#arguments

The primary intent of check() really is to show you what R CMD check would see, if it were run by, say, CRAN. The optional execution of document() is really a convenience or courtesy for interactive development. I'd say the devtools workflow assumes you are also running document() by itself fairly frequently, which will also surface these items for attention.

Do you have anything to add or a different take @gaborcsardi or @hadley? I do note that this pertains to relatively new opinions expressed by roxygen2.