From 930ba70a9c194a509a898cac1ffdfe663b7efb65 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 15:59:06 +0200 Subject: [PATCH 01/18] bump version --- DESCRIPTION | 2 +- NEWS.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 9a73ed4..0b13590 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: touchstone Title: Benchmark Different Git Refs in One Run -Version: 0.0.0.9001 +Version: 0.1.0 Authors@R: c(person(given = "Lorenz", family = "Walthert", diff --git a/NEWS.md b/NEWS.md index d8d06af..1c3662a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -# touchstone 0.1.0-rc +# touchstone 0.1.0 This is the initial CRAN release of {touchstone}. Please see the README.md on how to get started. From 39be227cd3fa814dbc3acb0e6b10d0ffb9c6eea4 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 16:23:52 +0200 Subject: [PATCH 02/18] expand title --- DESCRIPTION | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 0b13590..f4ff3ea 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Type: Package Package: touchstone -Title: Benchmark Different Git Refs in One Run +Title: Benchmarking Different 'git' Refs with Confidence in One Go Version: 0.1.0 Authors@R: c(person(given = "Lorenz", @@ -15,7 +15,8 @@ Authors@R: Description: A common problem of benchmarking in continuous integration is that the computational power of the virtual machines that run the job varies over time. This package allows users to benchmark two git refs - of the same repo in a random sequence for better comparison. + of the same repo in a random sequence for better comparison, booth + locally and via 'GitHub Actions'. License: MIT + file LICENSE URL: https://github.com/lorenzwalthert/touchstone, https://lorenzwalthert.github.io/touchstone From 044842144e405c4a083b9bc66e607287cdb8c798 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 16:28:08 +0200 Subject: [PATCH 03/18] add CRAN comments --- .Rbuildignore | 1 + DESCRIPTION | 1 - cran-comments.md | 12 ++++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 cran-comments.md diff --git a/.Rbuildignore b/.Rbuildignore index ec14350..f1f34d0 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -10,3 +10,4 @@ ^pkgdown$ ^doc$ ^Meta$ +^cran-comments\.md$ diff --git a/DESCRIPTION b/DESCRIPTION index f4ff3ea..eae9717 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -49,7 +49,6 @@ VignetteBuilder: Config/testthat/edition: 3 Config/testthat/parallel: true Encoding: UTF-8 -LazyData: true Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "collate", if (rlang::is_installed("pkgapi")) "pkgapi::api_roclet" else { warning("Please install r-lib/pkgapi to make sure the file API is kept diff --git a/cran-comments.md b/cran-comments.md new file mode 100644 index 0000000..a3ceee6 --- /dev/null +++ b/cran-comments.md @@ -0,0 +1,12 @@ +## Test environments + +* local R installation, R 4.1.0 +* ubuntu 18.04 (on GitHub Actions), R 4.1.0 as well as R devel. +* Windows Server 10 (on GitHub Actions): R 4.1.0. +* win-builder (devel) + +## R CMD check results + +0 errors | 0 warnings | 1 note + +* This is a new release. From 3406359a39fcb558805fea3416ea4bd796c3178e Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 16:58:29 +0200 Subject: [PATCH 04/18] fix r cmd check issues --- DESCRIPTION | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index eae9717..315a013 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Type: Package Package: touchstone -Title: Benchmarking Different 'git' Refs with Confidence in One Go +Title: Benchmarking Different 'Git' Refs with Confidence in One Go Version: 0.1.0 Authors@R: c(person(given = "Lorenz", @@ -14,12 +14,13 @@ Authors@R: comment = c(ORCID = "0000-0002-7281-3989"))) Description: A common problem of benchmarking in continuous integration is that the computational power of the virtual machines that run the job - varies over time. This package allows users to benchmark two git refs - of the same repo in a random sequence for better comparison, booth - locally and via 'GitHub Actions'. + varies over time. This package runs benchmarking code of two 'Git' + refs of the same repository in a random sequence for better + comparison, either locally or via 'GitHub Actions' and outputs a + confidence interval for the relative difference. License: MIT + file LICENSE URL: https://github.com/lorenzwalthert/touchstone, - https://lorenzwalthert.github.io/touchstone + https://lorenzwalthert.github.io/touchstone/ BugReports: https://github.com/lorenzwalthert/touchstone/issues Imports: bench, From cda4c5b537d8fe65ffb5e81b586dab6a872c13a5 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 18:07:30 +0200 Subject: [PATCH 05/18] trailing / --- DESCRIPTION | 4 ++-- man/touchstone-package.Rd | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 315a013..c425a74 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -19,9 +19,9 @@ Description: A common problem of benchmarking in continuous integration is comparison, either locally or via 'GitHub Actions' and outputs a confidence interval for the relative difference. License: MIT + file LICENSE -URL: https://github.com/lorenzwalthert/touchstone, +URL: https://github.com/lorenzwalthert/touchstone/, https://lorenzwalthert.github.io/touchstone/ -BugReports: https://github.com/lorenzwalthert/touchstone/issues +BugReports: https://github.com/lorenzwalthert/touchstone/issues/ Imports: bench, callr, diff --git a/man/touchstone-package.Rd b/man/touchstone-package.Rd index 67b8dc9..70e8a65 100644 --- a/man/touchstone-package.Rd +++ b/man/touchstone-package.Rd @@ -4,19 +4,21 @@ \name{touchstone-package} \alias{touchstone} \alias{touchstone-package} -\title{touchstone: Benchmark Different Git Refs in One Run} +\title{touchstone: Benchmarking Different 'Git' Refs with Confidence in One Go} \description{ A common problem of benchmarking in continuous integration is that the computational power of the virtual machines that run the job - varies over time. This package allows users to benchmark two git refs - of the same repo in a random sequence for better comparison. + varies over time. This package runs benchmarking code of two 'Git' + refs of the same repository in a random sequence for better + comparison, either locally or via 'GitHub Actions' and outputs a + confidence interval for the relative difference. } \seealso{ Useful links: \itemize{ - \item \url{https://github.com/lorenzwalthert/touchstone} - \item \url{https://lorenzwalthert.github.io/touchstone} - \item Report bugs at \url{https://github.com/lorenzwalthert/touchstone/issues} + \item \url{https://github.com/lorenzwalthert/touchstone/} + \item \url{https://lorenzwalthert.github.io/touchstone/} + \item Report bugs at \url{https://github.com/lorenzwalthert/touchstone/issues/} } } From 0476e7e92c04ab2000370f1a65718d655163e9c9 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 18:35:25 +0200 Subject: [PATCH 06/18] update CRAN comments --- cran-comments.md | 8 +++++++- inst/WORDLIST | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cran-comments.md b/cran-comments.md index a3ceee6..a49dc69 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -7,6 +7,12 @@ ## R CMD check results -0 errors | 0 warnings | 1 note +0 errors | 0 warnings | 2 note * This is a new release. +* Found the following calls to attach(): + File 'touchstone/R/core.R': + attach(loadNamespace("touchstone"), name = new_name) + See section 'Good practice' in '?attach'. + +For the latter, I followed the good practices. diff --git a/inst/WORDLIST b/inst/WORDLIST index 00c8433..14c390d 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -72,6 +72,7 @@ libgit libpaths linux listWorkflowRunArtifacts +loadNamespace lorenz lorenzwalthert magrittr From 60cdba548bef8053731fb2e8ea2f1d0991ba0066 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 18:44:49 +0200 Subject: [PATCH 07/18] improve explanation --- cran-comments.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cran-comments.md b/cran-comments.md index a49dc69..8ec274d 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -15,4 +15,9 @@ attach(loadNamespace("touchstone"), name = new_name) See section 'Good practice' in '?attach'. -For the latter, I followed the good practices. +For the latter, I followed the good practices. My package needs to be invoked +from a separate R process (via the R package callr) because library paths +cannot be cleanly removed within an R session and this function requires that +temporarily. Alternatively, I could have exported `exprs_eval` and called it +from that sub-process with `::`, but I prefer not to export it since it's not a +user-facing function. From 4d91522adcc3a4fe0851de66d36f2ab7b6661e68 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 9 Jun 2021 22:28:05 +0200 Subject: [PATCH 08/18] fix redirects --- README.Rmd | 4 ++-- README.md | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/README.Rmd b/README.Rmd index 27d4162..0030470 100644 --- a/README.Rmd +++ b/README.Rmd @@ -3,7 +3,7 @@ output: github_document --- [![Lifecycle: -experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://www.tidyverse.org/lifecycle/#experimental) +experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://lifecycle.r-lib.org/articles/stages.html#experimental) [![R build status](https://github.com/lorenzwalthert/touchstone/workflows/R-CMD-check/badge.svg)](https://github.com/lorenzwalthert/touchstone/actions) @@ -52,7 +52,7 @@ package developers. The following insights were the motivation: least when benchmark results don't vary significantly (> 50%). - **Dependencies should be identical across versions:** R and package versions - of dependencies must be fixed via [RSPM](http://packagemanager.rstudio.com) to + of dependencies must be fixed via [RSPM](https://packagemanager.rstudio.com/client/#/) to allow as much continuation as possible anyways. Changing the timestamp of RSPM can happen in PRs that are only dedicated to dependency updates. diff --git a/README.md b/README.md index 5010265..f43ed43 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ [![Lifecycle: -experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://www.tidyverse.org/lifecycle/#experimental) +experimental](https://img.shields.io/badge/lifecycle-experimental-orange.svg)](https://lifecycle.r-lib.org/articles/stages.html#experimental) [![R build status](https://github.com/lorenzwalthert/touchstone/workflows/R-CMD-check/badge.svg)](https://github.com/lorenzwalthert/touchstone/actions) @@ -46,9 +46,10 @@ motivation: - **Dependencies should be identical across versions:** R and package versions of dependencies must be fixed via - [RSPM](http://packagemanager.rstudio.com) to allow as much - continuation as possible anyways. Changing the timestamp of RSPM can - happen in PRs that are only dedicated to dependency updates. + [RSPM](https://packagemanager.rstudio.com/client/#/) to allow as + much continuation as possible anyways. Changing the timestamp of + RSPM can happen in PRs that are only dedicated to dependency + updates. - **Pull requests are a natural unit for measuring speed:** Linking benchmarking to pull requests make sense because you can easily From 38953b6bd46cbb563c75806e5417690d50373b71 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 14 Jun 2021 17:04:36 +0200 Subject: [PATCH 09/18] bump version --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index c425a74..d27ba42 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: touchstone Title: Benchmarking Different 'Git' Refs with Confidence in One Go -Version: 0.1.0 +Version: 0.1.1 Authors@R: c(person(given = "Lorenz", family = "Walthert", From bf2cba5541df42461945ff4b3a264b3dedc959ee Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Fri, 9 Jul 2021 12:30:46 +0200 Subject: [PATCH 10/18] cran review: use fast way to check --- R/utils.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index e984601..6470455 100644 --- a/R/utils.R +++ b/R/utils.R @@ -179,7 +179,7 @@ is_installed <- function(path_pkg = ".") { pkg_name <- unname(read.dcf(path_desc)[, "Package"]) list( name = pkg_name, - installed = pkg_name %in% rownames(utils::installed.packages()) + installed = rlang::is_installed(pkg_name) ) } From baa6620bd05c2b1bc2ae7c0178926fb19618c362 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Fri, 9 Jul 2021 18:50:38 +0200 Subject: [PATCH 11/18] document return value of internal functions --- R/analyze.R | 4 ++++ R/core.R | 4 ++++ R/testing.R | 4 ++++ R/utils.R | 15 ++++++++++++--- man/assert_no_global_installation.Rd | 6 ++++++ man/benchmark_run_iteration.Rd | 3 +++ man/benchmark_run_ref_impl.Rd | 3 +++ man/benchmark_verbalize.Rd | 3 +++ man/exprs_eval.Rd | 3 +++ man/is_installed.Rd | 4 ++++ man/local_clean_touchstone.Rd | 3 +++ man/local_package.Rd | 3 +++ man/local_without_touchstone_lib.Rd | 3 +++ man/ref_upsample.Rd | 3 +++ man/touchstone_managers.Rd | 7 ++----- 15 files changed, 60 insertions(+), 8 deletions(-) diff --git a/R/analyze.R b/R/analyze.R index c381901..f14a938 100644 --- a/R/analyze.R +++ b/R/analyze.R @@ -96,6 +96,8 @@ benchmark_analyze <- function(benchmark, refs = c( #' #' `refs` must be passed because the order is relevant. #' @inheritParams benchmark_plot +#' @return +#' A character vector with the text that verbalizes the benchmark. #' @keywords internal benchmark_verbalize <- function(benchmark, timings, refs, ci) { tbl <- timings %>% @@ -160,6 +162,8 @@ confint_relative_get <- function(timings, refs, reference, ci) { #' @param timing a benchmark read with [benchmark_read()], column `name` must #' only contain one unique value. +#' @return +#' The function is called for its side effects and returns `NULL` (invisibly). #' @keywords internal benchmark_plot <- function(benchmark, timings) { timings %>% diff --git a/R/core.R b/R/core.R index 92479fd..9d4a98f 100644 --- a/R/core.R +++ b/R/core.R @@ -4,6 +4,8 @@ #' @param n Number of iterations to run a benchmark within an iteration. #' @param dots list of quoted expressions (length 1). #' @inheritParams benchmark_write +#' @return +#' A tibble with the benchmarks. #' @importFrom tibble lst tibble #' @keywords internal benchmark_run_iteration <- function(expr_before_benchmark, @@ -96,6 +98,8 @@ benchmark_run_ref <- function(expr_before_benchmark, #' #' @param path_pkg The path to the root of the package you want to benchmark. #' @inheritParams benchmark_run_iteration +#' @return +#' A tibble with the benchmarks. #' @keywords internal benchmark_run_ref_impl <- function(ref, block, diff --git a/R/testing.R b/R/testing.R index 7bab858..1e2d97c 100644 --- a/R/testing.R +++ b/R/testing.R @@ -4,6 +4,8 @@ #' @inheritParams withr::defer #' @family testers #' @keywords internal +#' @return +#' The output of `withr::defer()`, i.e. a list with `expr` and `envir`. local_clean_touchstone <- function(envir = parent.frame()) { withr::defer(touchstone_clear(all = TRUE), envir = envir) } @@ -27,6 +29,8 @@ path_temp_pkg <- function(name) { #' to validate if the installed package corresponds to source branch for #' testing. If `NULL`, nothing is written. #' @inheritParams withr::defer +#' @return +#' Character vector of length one with path to package. #' @family testers #' @keywords internal local_package <- function(pkg_name = fs::path_file(tempfile("pkg")), diff --git a/R/utils.R b/R/utils.R index 6470455..0e83646 100644 --- a/R/utils.R +++ b/R/utils.R @@ -7,7 +7,8 @@ NULL #' @describeIn touchstone_managers returns the directory where the touchstone database lives. #' @aliases touchstone_managers #' @return -#' Character vector of length one with th path to the touchstone directory. +#' Character vector of length one with the path to the touchstone directory (for +#' `dir_touchstone()`), path to the deleted files for (`touchstone_clear()`). #' @export dir_touchstone <- function() { getOption("touchstone.dir", "touchstone") @@ -44,8 +45,6 @@ path_touchstone_script <- function() { #' @aliases touchstone_managers #' @param all Whether to clear the whole touchstone directory or just the #' records sub directory. -#' @return -#' The deleted paths (invisibly). #' @export touchstone_clear <- function(all = FALSE) { paths <- fs::path(dir_touchstone(), if (!all) c("records", "lib") else "") @@ -90,6 +89,8 @@ exprs_eval <- function(..., env = parent.frame()) { #' always perform worse than the second, so the order within the blocks must be #' random. #' @keywords internal +#' @return +#' A tibble with columns `block` and `ref`. ref_upsample <- function(ref, n = 20) { purrr::map_dfr( rlang::seq2(1, n), @@ -143,6 +144,8 @@ local_git_checkout <- function(branch, #' #' Advantages: Keep benchmarked repo in touchstone library only. #' @keywords internal +#' @return +#' The old library paths (invisibly). local_without_touchstone_lib <- function(path_pkg = ".", envir = parent.frame()) { all <- normalizePath(.libPaths()) is_touchstone <- fs::path_has_parent( @@ -155,6 +158,9 @@ local_without_touchstone_lib <- function(path_pkg = ".", envir = parent.frame()) #' Make sure there is no installation of the package to benchmark in the global #' package library +#' @param path_pkg The path to the package to check. +#' @return +#' `TRUE` invisibly or an error if there is a global installation. #' @keywords internal assert_no_global_installation <- function(path_pkg = ".") { local_without_touchstone_lib() @@ -173,6 +179,9 @@ assert_no_global_installation <- function(path_pkg = ".") { #' Check if a package is installed and unloading it +#' @return +#' A list with `name` of the package to check and `installed`, a boolean to +#' indicate if the package is installed or not. #' @keywords internal is_installed <- function(path_pkg = ".") { path_desc <- fs::path(path_pkg, "DESCRIPTION") diff --git a/man/assert_no_global_installation.Rd b/man/assert_no_global_installation.Rd index 7c2c885..7143fcc 100644 --- a/man/assert_no_global_installation.Rd +++ b/man/assert_no_global_installation.Rd @@ -7,6 +7,12 @@ package library} \usage{ assert_no_global_installation(path_pkg = ".") } +\arguments{ +\item{path_pkg}{The path to the package to check.} +} +\value{ +\code{TRUE} invisibly or an error if there is a global installation. +} \description{ Make sure there is no installation of the package to benchmark in the global package library diff --git a/man/benchmark_run_iteration.Rd b/man/benchmark_run_iteration.Rd index 74d9715..255743a 100644 --- a/man/benchmark_run_iteration.Rd +++ b/man/benchmark_run_iteration.Rd @@ -25,6 +25,9 @@ commit, tag, branch etc) of the benchmarking.} \item{n}{Number of iterations to run a benchmark within an iteration.} } +\value{ +A tibble with the benchmarks. +} \description{ Run a benchmark iteration } diff --git a/man/benchmark_run_ref_impl.Rd b/man/benchmark_run_ref_impl.Rd index a5a9e42..ed82c71 100644 --- a/man/benchmark_run_ref_impl.Rd +++ b/man/benchmark_run_ref_impl.Rd @@ -19,6 +19,9 @@ the benchmark is ran, will be evaluated with \code{\link[=exprs_eval]{exprs_eval \item{path_pkg}{The path to the root of the package you want to benchmark.} } +\value{ +A tibble with the benchmarks. +} \description{ Checkout a branch from a repo and run an iteration } diff --git a/man/benchmark_verbalize.Rd b/man/benchmark_verbalize.Rd index acf13ba..7e384f5 100644 --- a/man/benchmark_verbalize.Rd +++ b/man/benchmark_verbalize.Rd @@ -6,6 +6,9 @@ \usage{ benchmark_verbalize(benchmark, timings, refs, ci) } +\value{ +A character vector with the text that verbalizes the benchmark. +} \description{ \code{refs} must be passed because the order is relevant. } diff --git a/man/exprs_eval.Rd b/man/exprs_eval.Rd index 79d5fdd..89e5f36 100644 --- a/man/exprs_eval.Rd +++ b/man/exprs_eval.Rd @@ -15,6 +15,9 @@ exprs_eval(..., env = parent.frame()) \value{ The quoted input (invisibly). } +\value{ +The input, parsed and evaluated. +} \description{ Evaluate an expression for sideeffects } diff --git a/man/is_installed.Rd b/man/is_installed.Rd index a2b4807..86a366c 100644 --- a/man/is_installed.Rd +++ b/man/is_installed.Rd @@ -6,6 +6,10 @@ \usage{ is_installed(path_pkg = ".") } +\value{ +A list with \code{name} of the package to check and \code{installed}, a boolean to +indicate if the package is installed or not. +} \description{ Check if a package is installed and unloading it } diff --git a/man/local_clean_touchstone.Rd b/man/local_clean_touchstone.Rd index 5921fd3..1091aa2 100644 --- a/man/local_clean_touchstone.Rd +++ b/man/local_clean_touchstone.Rd @@ -11,6 +11,9 @@ local_clean_touchstone(envir = parent.frame()) Typically, this should be either the current environment or a parent frame (accessed through \code{\link[=parent.frame]{parent.frame()}}).} } +\value{ +The output of \code{withr::defer()}, i.e. a list with \code{expr} and \code{envir}. +} \description{ Deletes \code{\link[=dir_touchstone]{dir_touchstone()}} when the local frame is destroyed. } diff --git a/man/local_package.Rd b/man/local_package.Rd index 7ae77c6..ec34e08 100644 --- a/man/local_package.Rd +++ b/man/local_package.Rd @@ -28,6 +28,9 @@ set to the package root.} Typically, this should be either the current environment or a parent frame (accessed through \code{\link[=parent.frame]{parent.frame()}}).} } +\value{ +Character vector of length one with path to package. +} \description{ Creates a package in a temporary directory and sets the working directory until the local frame is destroyed. diff --git a/man/local_without_touchstone_lib.Rd b/man/local_without_touchstone_lib.Rd index 0210c9e..5111359 100644 --- a/man/local_without_touchstone_lib.Rd +++ b/man/local_without_touchstone_lib.Rd @@ -12,6 +12,9 @@ local_without_touchstone_lib(path_pkg = ".", envir = parent.frame()) \item{envir}{The environment that triggers the deferred action on destruction.} } +\value{ +The old library paths (invisibly). +} \description{ This is useful in conjunction with \code{\link[=run_script]{run_script()}}. } diff --git a/man/ref_upsample.Rd b/man/ref_upsample.Rd index 0ff9058..9e37725 100644 --- a/man/ref_upsample.Rd +++ b/man/ref_upsample.Rd @@ -6,6 +6,9 @@ \usage{ ref_upsample(ref, n = 20) } +\value{ +A tibble with columns \code{block} and \code{ref}. +} \description{ A block is a permutation of all unique elements in \code{ref}. Then, we sample \code{n} blocks. This is better than repeating one sample a certain number of diff --git a/man/touchstone_managers.Rd b/man/touchstone_managers.Rd index 78cf4ba..04e25b8 100644 --- a/man/touchstone_managers.Rd +++ b/man/touchstone_managers.Rd @@ -18,11 +18,8 @@ path_pr_comment() records sub directory.} } \value{ -Character vector of length one with th path to the touchstone directory. - -The deleted paths (invisibly). - -Character vector of length one with the path to the pr comment. +Character vector of length one with the path to the touchstone directory (for +\code{dir_touchstone()}), path to the deleted files for (\code{touchstone_clear()}). } \description{ Utilities to manage the touchstone database. From 08671acc988edb107c8674fd10eb6fcbb52dd6db Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 10 Jul 2021 09:06:39 +0200 Subject: [PATCH 12/18] use smarter way to check if package is installed --- R/utils.R | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index 0e83646..718d062 100644 --- a/R/utils.R +++ b/R/utils.R @@ -188,7 +188,22 @@ is_installed <- function(path_pkg = ".") { pkg_name <- unname(read.dcf(path_desc)[, "Package"]) list( name = pkg_name, - installed = rlang::is_installed(pkg_name) + installed = is_installed_impl(pkg_name) + ) +} + +# When a package is removed from the library with [remove.packages()] and then +# loaded e.g. with [devtools::load_all()], +# [rlang::is_installed()] and similar returns `TRUE`, +# `pkg_name %in% installed.packages()` `FALSE` (but CRAN does not want to see +# `installed.packages()` used since it may be slow). +is_installed_impl <- function(pkg_name) { + rlang::with_handlers( + { + find.package(pkg_name, lib.loc = .libPaths()) + TRUE + }, + error = ~FALSE ) } From 3df8b4846e32ca622f3ac397c6d04abeafb1c5f3 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 10 Jul 2021 09:10:29 +0200 Subject: [PATCH 13/18] version bump --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index d27ba42..b240ad1 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: touchstone Title: Benchmarking Different 'Git' Refs with Confidence in One Go -Version: 0.1.1 +Version: 0.1.2 Authors@R: c(person(given = "Lorenz", family = "Walthert", From 081618e167d2eaeec0f8ca02c802e67e51972a24 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 16 Oct 2021 12:43:28 +0200 Subject: [PATCH 14/18] update cran comments --- cran-comments.md | 24 ++++++++++++++++++++++ inst/WORDLIST | 52 +++++++++++++++++++++++++----------------------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/cran-comments.md b/cran-comments.md index 8ec274d..ad112b6 100644 --- a/cran-comments.md +++ b/cran-comments.md @@ -1,3 +1,6 @@ +This is a re-submission based on the feedback from Gregor Seyer received on June +18, 2021. + ## Test environments * local R installation, R 4.1.0 @@ -21,3 +24,24 @@ cannot be cleanly removed within an R session and this function requires that temporarily. Alternatively, I could have exported `exprs_eval` and called it from that sub-process with `::`, but I prefer not to export it since it's not a user-facing function. + + +In addition, I addressed all comments by Gregor Seyer. + + +* add \value to .Rd files regarding exported methods. + +> I already had that for all exported methods. You are referring to unexported +methods with the keyword internal. I think the check should be adapted to only +require return value documentation for exported methods (or adapt the standard +text above to include all documented objects). + +* You have examples for unexported functions. `is_installed()`. + +> This is a false positive. I was using `rlang::is_installed()` in an example, +not the function `is_installed()` from my own package. Nevertheless, I worked +around this. + +* You are using installed.packages(): + +> Thanks, I use your suggested way of checking now if a package is installed. diff --git a/inst/WORDLIST b/inst/WORDLIST index 14c390d..0efb64d 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -1,37 +1,16 @@ -BugReports -CMD -Config -Jens -LIBS -LazyData -Lifecycle -MERCHANTABILITY -NONINFRINGEMENT -ORCID -PRs -README -RHUB -RSPM -Rds -Roxygen -RoxygenNote -Rscript -SHA -Sys -VMs -VignetteBuilder -Walthert -Wujciak api aut ba bcea benchmarked benchmarking +BugReports callr ci cli +CMD config +Config cr cran cre @@ -52,24 +31,29 @@ fromJson fs gcc gert -getRversion getenv +getRversion getwd ggplot github gitignore +Gregor hashFiles href https icloud io jacob +Jens json knitr +LazyData lenth libcurl libgit libpaths +LIBS +Lifecycle linux listWorkflowRunArtifacts loadNamespace @@ -78,36 +62,50 @@ lorenzwalthert magrittr matchArtifact md +MERCHANTABILITY mkdir netlify +NONINFRINGEMENT openssl orcid +ORCID os packagemanager pkgapi pkgdown prepending +PRs purrr rc +Rds readFileSync +README repo +RHUB rlang rmarkdown roclet roclets roxygen +Roxygen +RoxygenNote +Rscript rspm +RSPM rstudio runif saveRDS seealso sep +Seyer +SHA sideeffects sprintf styfle styler sublicense sudo +Sys sysreq sysreqs tada @@ -121,9 +119,13 @@ ubuntu usethis va vctrs +VignetteBuilder +VMs walthert +Walthert withr writeFileSync writeLines wujciak +Wujciak yourpkg From 5ed2e595d6a537afbfec433f5dc38e46d3cd48d4 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 10 Jul 2021 10:53:18 +0200 Subject: [PATCH 15/18] ignore touchstone dir --- .Rbuildignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.Rbuildignore b/.Rbuildignore index f1f34d0..cd4b055 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -11,3 +11,4 @@ ^doc$ ^Meta$ ^cran-comments\.md$ +^touchstone$ From 9d334fc11edbd871d5b64f8b43323b53bda9a2d8 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 10 Jul 2021 11:07:37 +0200 Subject: [PATCH 16/18] fix version in NEWS.md --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 1c3662a..bf78a2c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -# touchstone 0.1.0 +# touchstone 0.1.2 This is the initial CRAN release of {touchstone}. Please see the README.md on how to get started. From 75b3665de5bfde26e553949ec6e6ae93c6421ba6 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 7 Aug 2021 16:06:52 +0200 Subject: [PATCH 17/18] new attempt at cran release --- DESCRIPTION | 2 +- NEWS.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index b240ad1..568d429 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: touchstone Title: Benchmarking Different 'Git' Refs with Confidence in One Go -Version: 0.1.2 +Version: 0.1.3 Authors@R: c(person(given = "Lorenz", family = "Walthert", diff --git a/NEWS.md b/NEWS.md index bf78a2c..5c1d632 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -# touchstone 0.1.2 +# touchstone 0.1.3 This is the initial CRAN release of {touchstone}. Please see the README.md on how to get started. From da308845b447adf0e4425009387a36db400c2ad6 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sat, 16 Oct 2021 13:02:43 +0200 Subject: [PATCH 18/18] fix roxygen --- R/testing.R | 2 +- R/utils.R | 7 +++---- man/exprs_eval.Rd | 3 --- man/touchstone_managers.Rd | 5 +++-- 4 files changed, 7 insertions(+), 10 deletions(-) diff --git a/R/testing.R b/R/testing.R index 1e2d97c..e2d3c8c 100644 --- a/R/testing.R +++ b/R/testing.R @@ -58,7 +58,7 @@ local_package <- function(pkg_name = fs::path_file(tempfile("pkg")), gert::git_add("R/") gert::git_commit("[init]") branches <- gert::git_branch_list() %>% - dplyr::pull(name) %>% + dplyr::pull(.data$name) %>% dplyr::setdiff(branches, .) purrr::walk(branches, gert::git_branch_create) withr::defer(unlink(path), envir = envir) diff --git a/R/utils.R b/R/utils.R index 718d062..b482d4c 100644 --- a/R/utils.R +++ b/R/utils.R @@ -8,7 +8,8 @@ NULL #' @aliases touchstone_managers #' @return #' Character vector of length one with the path to the touchstone directory (for -#' `dir_touchstone()`), path to the deleted files for (`touchstone_clear()`). +#' `dir_touchstone()`), path to the deleted files (for `touchstone_clear()`) or +#' path to the file containing the pull request comment (for `path_pr_comment()`). #' @export dir_touchstone <- function() { getOption("touchstone.dir", "touchstone") @@ -334,10 +335,8 @@ get_asset_dir <- function(ref, verb = "find") { } -#' @describeIn touchstone_managers returns the path to the file containing the pr comment. +#' @describeIn touchstone_managers Returns the path to the file containing the pull request comment. #' @aliases touchstone_managers -#' @return -#' Character vector of length one with the path to the pr comment. #' @export #' @seealso [pr_comment] path_pr_comment <- function() { diff --git a/man/exprs_eval.Rd b/man/exprs_eval.Rd index 89e5f36..79d5fdd 100644 --- a/man/exprs_eval.Rd +++ b/man/exprs_eval.Rd @@ -15,9 +15,6 @@ exprs_eval(..., env = parent.frame()) \value{ The quoted input (invisibly). } -\value{ -The input, parsed and evaluated. -} \description{ Evaluate an expression for sideeffects } diff --git a/man/touchstone_managers.Rd b/man/touchstone_managers.Rd index 04e25b8..e073c2f 100644 --- a/man/touchstone_managers.Rd +++ b/man/touchstone_managers.Rd @@ -19,7 +19,8 @@ records sub directory.} } \value{ Character vector of length one with the path to the touchstone directory (for -\code{dir_touchstone()}), path to the deleted files for (\code{touchstone_clear()}). +\code{dir_touchstone()}), path to the deleted files (for \code{touchstone_clear()}) or +path to the file containing the pull request comment (for \code{path_pr_comment()}). } \description{ Utilities to manage the touchstone database. @@ -30,7 +31,7 @@ Utilities to manage the touchstone database. \item \code{touchstone_clear}: clears the touchstone database. -\item \code{path_pr_comment}: returns the path to the file containing the pr comment. +\item \code{path_pr_comment}: Returns the path to the file containing the pull request comment. }} \seealso{