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

Fix warning in p_install_gh when installing multiple packages #122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LoHertel
Copy link

This PR addresses the warning message mentioned in #118 when installing multiple packages with p_install_gh().

The code in R/p_install_gh.R#L24 throws a warning message when passing a vector of repository names to the package argument: p_install_gh(c('path/package', 'path2/repo'))

Warning message:
if (p_loaded(char = package)):
  the condition has length > 1 and only the first element will be used

I propose the following changes:

  • If clause should check for an array of booleans instead of a single boolean.
  • Because p_loaded() is using the package name to check, whether that package is loaded, the package name should be extracted from the Github path beforehand with parse_git_repo(). (Because p_install_gh() passes a Github path (e.g. 'trinker/pacman') to p_loaded(). p_loaded() always returns FALSE for Github paths.)
  • Update the code of parse_git_repo() in R/utils.R#L194 from the remotes package. It would act as an interface to call either remotes::parse_github_url() or remotes::parse_repo_spec(). The code for parse_git_repo() is taken from remotes/R/parse-git.R#L97. parse_git_repo() depends on viapply(), therefore it is included from remotes/R/utils.R#L8 as well.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 0.0% when pulling 44ceb33 on LoHertel:fix-install-gh into fc09ccd on trinker:master.

@LoHertel LoHertel changed the title Fix that p_install_gh installes multiple packages without warning Fix warning in p_install_gh when installing multiple packages May 22, 2019
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

Successfully merging this pull request may close these issues.

2 participants