Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upWrap pre-built pdf vignettes #1004
Comments
This comment has been minimized.
This comment has been minimized.
Sounds good to me! Just to check, have we ever had an issue in Rcpp that was caught during building of vignettes, but not during running of the test suite for some reason? As far as I can recall, this has never occurred but your memory may be better than mine. |
This comment has been minimized.
This comment has been minimized.
I like the empirical angle. And just like you I also cannot think of an issue it triggered. I think there was general test fickleness. I recall that for quite some time (when files were in Rnw before being converted to Rmd) they had code that "looked like" evaluation snippets but they were all set to false anyway... I guess having the vignettes fail helps for issue like the recent pinp issue (which at the end an error on my end on the latex side tickled by a new texlive) so in that sense reducing the test surface is not nice. But then again it also does not help that CRAN doesn't even have enough time for all our tests.---many are conditioned away unless a 'yes we really want to test' variable is set as our test runner does , or as we do with the a.b.c.d scheme which tests whereas releases in a.b.c form don't. Long story short I also don't recall the vignettes saving us. These days the real test are the reverse depends anyway. |
Our vignettes do not change that much, and do not contain much "test" code that adds to the decent coverage we have from unit testing.
But building all nine pdf vignettes takes on the order of two minutes. Given that the content does not really change all that much, the time may not be well spent.
Moreover, the toolchain changes every now and then. Recently we had an issue in the pinp package (providing the Markdown/LaTeX parts); rmarkdown or pandoc also change over the years. We may as well shield Rcpp from this.
The branch vignettes_premade has a set of changes which uses a standard trick of having a minimal Sweave/LaTeX vignette "including" another pdf file. I added Makefiles to make the actual 'underlying' vignettes from rmd, and the wrapper vignettes.
If there are no objections, I plan to make this a pr in a day or two (I need to add wrappers for two of the nine vignettes) and then merge.
Comments welcome and appreciated, as always!