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

Pass unnamed parameters to install_deps from install for more consistent behavior with unnamed parameters #402

Merged

Conversation

siyangli32
Copy link
Contributor

In the current code, unnamed parameters are not being passed and respected in install_deps while being respected in the safe_install_package itself. This leads to inconsistent behavior with unnamed parameters and how they are treated with regards to the package installation vs. the installation of the package dependencies.

I am not sure if the exclusion of the unnamed parameters being passed into install_deps was intentional. The documentation on the parameters does not seem to be explicit about this nuance so I suspect this was a bug.

For more context, here is our specific use case and why this change is important.

  1. We are calling remotes through devtools
  2. We are using devtools to install packages into app-specific library folders as part of our build and deployment process (our app includes R integration)
  3. devtools and other build-related packages will be installed in a build machine-level library
  4. We will need to have devtools target different library from the one it is loaded from to install app-specific R dependencies
  5. This bug is preventing us from having that configurability (which it seems to offer from the documentation)

Happy to add more clarification if needed. Thank-you!

@jimhester
Copy link
Member

Can you please add a bullet to NEWS? It should briefly describe the change and end with (@yourname, #issuenumber).

@pommedeterresautee
Copy link
Contributor

@siyangli32 to pass CI test, you need to do a make all in remotes source folder (it will update some copy of some files)

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.

3 participants