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

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

Closed
Shoeboxam opened this issue Feb 14, 2024 · 4 comments

Comments

@Shoeboxam
Copy link

Shoeboxam commented Feb 14, 2024

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
Copy link
Member

jennybc commented Feb 15, 2024

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
Copy link
Author

Shoeboxam commented Feb 15, 2024

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
Copy link
Member

jennybc commented Feb 15, 2024

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.

@hadley
Copy link
Member

hadley commented Jul 26, 2024

I'd say everything is behaving as expected here.

@hadley hadley closed this as completed Jul 26, 2024
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

No branches or pull requests

3 participants