Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch from language: script to language: r #232

Closed
3 of 8 tasks
lorenzwalthert opened this issue Mar 8, 2021 · 2 comments
Closed
3 of 8 tasks

switch from language: script to language: r #232

lorenzwalthert opened this issue Mar 8, 2021 · 2 comments

Comments

@lorenzwalthert
Copy link
Owner

lorenzwalthert commented Mar 8, 2021

With the latest release, R is officially supported in pre-commit 馃帀. This means we can proceed here:

  • remove the warning about dependency management. add note that this repo uses r and isolates dependencies.
  • in the supported hooks registry of pre-commit, we should change the language to r.
  • we should update the pre-commit docs to reflect Use more common package definition for R聽pre-commit/pre-commit#1898 and Avoid warnings with R hooks when renv version don't match聽pre-commit/pre-commit#1841.
  • we must add a renv.lock and remove warnings on desc / usethis::use_package(). Depending on whether or not we rely on renv/activate.R, we must also add this.
  • we should test the hooks with renv versions and the utilities with latest CRAN.
  • we need a mechanism to periodically update the versions of the packages in renv.lock, probably in sync with the tag updates for the repo and the package version in DESCRIPTION. -> precommit:::release_gh().
  • test if new R package works with old hook versions
  • Blog post: Explain that renv is not a renv in the project directory. Explain upgrading.
@lorenzwalthert
Copy link
Owner Author

lorenzwalthert commented Mar 18, 2021

Our package has, as the README denotes, two purposes:

  • provide a set of useful hooks.
  • providing usethis like access to pre-commit.

With R as a supported language, we can isolate these two more. Maybe, one day, we'll even have separate repos for different hooks (i.e. place a .pre-commit-hooks.yaml in r-lib/styler and friends). So:

Hook tests
For hook scripts (and dependencies), we should test versions as denoted in renv.lock. We don't need the pre-commit executable. We should test on all platforms. We don't need r cmd check.

Usethis-like accessors
For usethis-like functionality, we should test CRAN with dependencies. We need the pre-commit executable and should test all combinations of installation methods and platfroms. We don't need r cmd check.

On CRAN, we need to test hooks with hook scripts and usethis-like functionality with CRAN dependencies as part of R CMD check on all platforms. Basically the second paragraph above, but skipping some tests. We can hence implement that in 2 workflows.

Using renv also implies that we no longer have to list dependencies of hooks in DESCRIPTION, which will make installation of the R package faster (and hook installation slower, once). However, if we want to check hooks on CRAN, we must install these dependencies, so probably we'll move all of these to Suggests. This means we probably should derive the hook dependencies to be listed in renv.lock from the hook scripts directly, and not from DESCRIPTION anymore. But for the hook scripts that call {precommit} hook script helpers, I am not sure renv will properly detect.

-> Put dependencies of R code from R/ as Imports: in DESCRIPTION, dependencies from inst/bin (hooks) into Suggests:. Detect dependencies from hooks with renv and put them into renv.lock (or rely on unit tests to ensure all deps are there?). Or just all suggests except testing deps like testthat?

@lorenzwalthert
Copy link
Owner Author

Done with release v0.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant