-
Notifications
You must be signed in to change notification settings - Fork 48
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
Prompt user to install missing and outdated dependencies #184
Conversation
cb1ffc2
to
a823858
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in theory — I'll install the dev version when you merge this and see how it feels in practice.
To avoid prompting twice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess YMMV, but this seems a bit too aggressive to me. How about just printing out the outdated packages instead, once a session?
if (is_installed("pak")) { | ||
deps <- pak::pkg_deps(path, upgrade = FALSE) | ||
deps <- deps[deps$package %in% pkg, ] | ||
pak::pak(deps$ref, ask = FALSE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does two rounds of dependency lookups, which is not ideal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a fix to suggest? This is getting the refs of the package names in pkg
which are potentially Remotes refs, and then installing them. Is there a way to do these two steps in one go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just call
pkg_install("deps::.")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid having to reinstall dependencies from github when pak does not recognise that a local or dev install is sufficient.
For my usage at least it's very rare that I don't want to install the minimal requirements for Imports and Depends packages. Perhaps we could add an option to control this behaviour if it turns out too annoying? We could first try it out to see how it works in practice. |
Merging now so we can test it more easily. |
Depends on r-lib/rlang#1332.
Uses
rlang::check_installed()
to prompt users about missing dependencies. The customaction
callback uses pak if installed, or remotes if installed, and otherwiseinstall.packages()
.For outdated packages, the prompt is only triggered if the versions do not match. This way we avoid the large overhead of the dependency functions of pak and remotes. Also this avoids having to reinstall from github when pak or remotes does not recognise that a local or dev install is sufficient.