devtools
devtools copied to clipboard
use_tidy_dependencies
This uses use_tidy_dependencies()
, which most consequentially adds glue to Imports, and adds brings in import-standalone-purrr.R
To start to make use of these I've replaced most instances of paste0()
with glue()
. I've also used the purrr-like functionality to remove the DIY compact()
, replaced vcapply()
with map_chr()
and vapply()
calls with their map_
equivalent.
The addition of glue takes the number of non-default packages to 21 which triggers a NOTE about excessive dependencies. Although not used, I see that profvis, miniUI, and bench are included in Imports - presumably so the user has them installed so they can use them without fuss?
Looking at pak::pkg_deps_explain("devtools", "cli")
, I wonder if cli would be a good candidate to move to Suggests and use cli::
throughout? It has many dependency pathways to ensure that it will always be installed, and it is easy to qualify with its namespace (and appears to be mostly called that way already in devtools).
Address part of #2501
Thanks @jennybc! I think I've addressed your comments and suggestions.
Regarding the number of dependencies, I do think it feels a bit messy to sort of obfuscate by moving packages to Suggests to get around an arbitrary threshold. If there's a way to keep packages in Imports that should be there and not feel too much pain with CRAN submissions I think that would be ideal.
Assuming we give up on playing games with the dependencies, here's how it's handled in the tidyverse package:
https://github.com/tidyverse/tidyverse/blob/main/cran-comments.md
The same rationale applies here as well.
## R CMD check results
0 errors | 0 warnings | 1 notes
> checking package dependencies ... NOTE
Imports includes 29 non-default packages.
Importing from so many packages makes the package vulnerable to any of
them becoming unavailable. Move as many as possible to Suggests and
use conditionally.
This is a false positive - the majority of packages are maintained by me or my team, so there's little risk of them becoming unavailable.
Nice and simple, I like it. If we decide to go that route, should I look in Suggests and move to Imports those packages that are used unconditionally (i.e., without if (!requireNamespace("pkg")) ...
)?
If we decide to go that route, should I look in Suggests and move to Imports those packages that are used unconditionally (i.e., without if (!requireNamespace("pkg")) ...)?
That seems sensible. A good first step would be to determine which packages that applies to, if any, for concreteness.
There are three packages in Suggests that are called unconditionally in code: callr, httr, and rstudioapi.
All have quite strong indirect dependency relationships but I think are probably worth moving into Imports as they are widely used and pretty critical:
library(pak)
pkg_deps_explain("devtools", deps = "callr")
#> devtools -> pkgbuild -> callr
#> devtools -> pkgdown -> callr
#> devtools -> rcmdcheck -> callr
#> devtools -> rcmdcheck -> pkgbuild -> callr
#> devtools -> testthat -> callr
pkg_deps_explain("devtools", deps = "httr")
#> devtools -> pkgdown -> httr
pkg_deps_explain("devtools", deps = "rstudioapi")
#> devtools -> usethis -> gert -> rstudioapi
#> devtools -> usethis -> rstudioapi
Created on 2023-03-07 with reprex v2.0.2
Yes, so these seem to be the packages that were sort of artificially moved to Suggests in #2411 to avoid the NOTE. I think putting them back in Imports brings this PR to an internally consistent state.
I think I have two final question @jennybc:
- Should I run
use_latest_dependencies()
or leave as is? I set glue to be the current CRAN version but haven't touched the others. - I think I should update NEWS.md with this changes, even though they're not really user-facing?
Should I run use_latest_dependencies() or leave as is? I set glue to be the current CRAN version but haven't touched the others.
Yes, that seems the most appropriate thing to do in this PR where we embrace the meta-package nature of devtools.
I think I should update NEWS.md with this changes, even though they're not really user-facing?
I'm not entirely sure what we can say that's user-facing, but sure go ahead of add such a bullet and we can decide how it feels.
Ok done - I added a few bullets to NEWS.md. Let me know if it's too much inside baseball!
@hadley Would you like to offer an opinion?
@ateucher and I both think it makes sense to admit that devtools is a meta-package, similar to tidyverse. And, therefore, we should stop doing weird gymnastics to keep the dependencies below 20 and, instead, just do what makes sense.
This was triggered by doing use_tidy_dependencies()
and, specifically, adding glue.
But I do think it would be nice to be able to use glue internally and to put callr, httr, and rstudioapi back in Imports
which is where they really belong.
So I guess I have two questions:
Are you on board with this? Or at least neutral?
If so, do you agree that we should also put minimum versions to CRAN versions, similar to tidyverse?
If you're willing to argue with CRAN about this (if needed, which it probably won't be), go for it.
And yes, I do think you should bump all the versions.