nathaneastwood / poorman

A poor man's dependency free grammar of data manipulation
https://nathaneastwood.github.io/poorman/
Other
338 stars 15 forks source link

Leave out unnecessary packages from `Suggests` #112

Closed IndrajeetPatil closed 2 years ago

IndrajeetPatil commented 2 years ago

IINM, there is no source or test code that uses any of the functions from these packages, so including them here should be unnecessary.

nathaneastwood commented 2 years ago

This sounds sensible, though maybe we need to place pkgdown under Config/Needs/website: in the DESCRIPTION file? I'm not sure as I haven't touched pkgdown in a long time so I'd need to read the documentation.

codecov-commenter commented 2 years ago

Codecov Report

Merging #112 (20e4601) into master (d7d73db) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #112   +/-   ##
=======================================
  Coverage   93.53%   93.53%           
=======================================
  Files          58       58           
  Lines        1455     1455           
=======================================
  Hits         1361     1361           
  Misses         94       94           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

IndrajeetPatil commented 2 years ago

we need to place pkgdown under Config/Needs/website: in the DESCRIPTION file

I think this is necessary only if you are using a template package (cf. https://pkgdown.r-lib.org/articles/customise.html?q=Config/Needs/website#template-packages).

Screenshot 2022-08-17 at 15 10 30

E.g. {dplyr} uses template package for {pkgdown} website called {tidytemplate}:

https://github.com/tidyverse/dplyr/blob/71d223c07e56fb9225bd301dc77011e4f2fc0b20/_pkgdown.yml#L3-L4

And so specifies this in the DESCRIPTION file:

https://github.com/tidyverse/dplyr/blob/71d223c07e56fb9225bd301dc77011e4f2fc0b20/DESCRIPTION#L78-L82

Since you are not using a template package, this is not relevant.

nathaneastwood commented 2 years ago

Thanks very much @IndrajeetPatil. Looks like the tests all passed so I am happy to get this merged :) and of course, reducing dependencies is always a priority ;)