Closed annakrystalli closed 4 years ago
Thank you for the observation and the PR!
The fix is fine, but maybe this is a good opportunity to get rid of add_desc_package
. A solution with with the desc package would probably be more fancy. And reliable. What do you think, @annakrystalli and @benmarwick?
While using the more formal desc
sounds sensible, I just had a look at it and I'm not sure how mature/stable it is. Seems like tests are failing and some important issues not seeming to be adressed (eg https://github.com/r-lib/desc/issues/68).
I guess as long as rrtools
test are passing you can ensure it's doing what you want but I leave it up to your discretion whether desc
is mature enough.
In any case, the reason I've spotted this is because I'm working on materials for a walkthrough of rrtools
I'm doing on 2019-09-17 (next week) at the UK RSE conference. So would be great to have this bug sorted by then if possible.
Thanks for spotting this, it's my mistake for assuming add_desc_package()
was vectorised without testing first! I think #97 is an excellent solution. I prefer not to add additional dependencies when we have a simple base fix like in this PR, so let's go ahead with that. I'm giving a full day workshop in this tomorrow in Japan, so I'm very grateful that you spotted this and made the PR! Hope your walkthough goes well @annakrystalli!
Ooooh good luck tomorrow!!
Please wait for some discussion of your report before making a Pull Request.
Describe the bug When running
use_analysis
, the DESCRIPTION file is not correctly populated with suggested packages. Instead what I get is:Tracing the error back through the source code I found that
add_desc_package()
does not vectorise well over a vector of packages supplied toname
(iec("devtools", "git2r")
), hence the awkward behaviour.To Reproduce
create_compendium("~/Desktop/test")
and then open the `DESCRIPTION file to see error.Expected behavior I would expect the following output instead:
The simplest way I can see to address the issue is to use
lapply
to vectorise the function over each suggested package. I'm making a short PR to that effect with added tests to check for it.Created on 2019-09-09 by the reprex package (v0.3.0)