Closed DavisVaughan closed 5 years ago
Wow tganks! This is really super helpful. Really happy to see you help us with best practices.
For the travis simplification, I would lean to making it in a separate PR, as it is probably going to require some iterations. I would like to keep the conda-based approach for the pure c++ xtensor-r package as it follows the same pattern as xtensor-python and xtensor-julia.
Sounds good!
I've added a few changes that should make this a bit more CRAN compatible + make it easier for people who use RStudio.
.Rproj
file. This is essentially harmless, and just let's users easily open the project with RStudio. When you open the project in RStudio using this, it sets up a few nice things for you (like setting the working directory to be the proj directory). If you don't want it, I can remove. This is a pretty common thing though.vendor
totools/vendor
. This is the standard place for extra configuration files (CRAN will complain about that file being at the "top level")man/figures/
. This is the generally approved place to store images. (CRAN will complain about it being at the "top level")src/.gitignore
to ignore some cpp artifacts that come from runningdevtools::load_all()
on the package locally, which will install it. This just helps to ensure we don't commit those to the repo. Most R packages using cpp do this.roxygen2
package is a really powerful and popular one. You add documentation notes with#'
in your R files directly, and these will generate documentation files and the NAMESPACE file for you by runningdevtools::document()
. I am now using that for the import ofRcpp::sourceCpp
and the dynamic registration. It's strange but generally done to put these registration commands inaaa.R
orzzz.R
or something similar.I also want to take a look at:
Revamping the test suite. By using the
tests/
folder (nottest/
), the tests will actually run when it is being checked by CRAN. This is preferable, and you wont have to trigger the checks on travis, it should happen automatically when checking.Simplifying travis. Now that this is really just an R package, I want to see if we can using the R travis builds rather than the cpp ones. It would probably simplify the builds a lot. The most complicated thing there will probably be ensuring that cpp14 is installed and is the default.