Skip to content

Commit f91bebe

Browse files
Merge pull request #328 from pwildenhain/update-contributing
- Update CONTRIBUTING.md (#328).
2 parents 0d6798c + 6087115 commit f91bebe

File tree

1 file changed

+14
-20
lines changed

1 file changed

+14
-20
lines changed

CONTRIBUTING.md

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
# General
22

33
Before making a pull request, discuss your ideas in an issue. This repo is
4-
experimental.
5-
This repo uses the [tic](https://github.com/ropenscilabs/tic) package for CI.
4+
maturing.
65

76
# Adding new hooks
87

@@ -13,34 +12,29 @@ the script, you can expect the passed command line arguments to be all options,
1312
finally the files that should be processed with the hook.
1413

1514
For the scripts to become a hook, they need to be *registered* in
16-
[`.pre-commit-hooks.yaml`](https://github.com/lorenzwalthert/precommit/blob/main/.pre-commit-hooks.yaml). As R is not currently a supported language of
17-
pre-commit (https://github.com/pre-commit/pre-commit/issues/926), most hooks use
18-
`language: script` and then a shebang in the `entrypoint` script.
15+
[`.pre-commit-hooks.yaml`](https://github.com/lorenzwalthert/precommit/blob/main/.pre-commit-hooks.yaml). As of pre-commit 2.11, R is a [supported language of
16+
pre-commit](https://pre-commit.com/#r). Hence, it should have `language: r` in `.pre-commit-hooks.yaml` and then (for compatibility) a shebang in the `entrypoint` script.
1917

2018
# Testing hooks
2119

2220
Hooks should be tested by checking both the positive outcome (hook passes) and
2321
the negative outcome (hook fails) by adding two `run_test()` statements to
24-
[`./tests/testthat/test-all.R`](https://github.com/lorenzwalthert/precommit/blob/main/tests/testthat/test-all.R). Look at existing examples and [the documentation
25-
of `run_test()`](https://lorenzwalthert.github.io/precommit/reference/run_test.html). Note that this won't actually use pre-commit. It will simply
26-
call the hook script the same way as pre-commit would, with the difference that
27-
the test uses a path to the `Rscript` executable whereas with pre-commit, the
28-
shebang is needed. Hence, omitting the shebang in your script will indicated
29-
passed tests, but this will be false positive. Also, there is no easy way to
30-
test if a hook is correctly registered in `.pre-commit-hooks.yaml`
31-
32-
For this reason, always test the hook manually end-to-end with
33-
`pre-commit try-repo` as described in the
34-
[documentation](https://pre-commit.com/#pre-commit-try-repo).
22+
[`./tests/testthat/test-hooks.R`](https://github.com/lorenzwalthert/precommit/blob/main/tests/testthat/test-hooks.R). Look at existing examples and [the documentation
23+
of `run_test()`](https://lorenzwalthert.github.io/precommit/reference/run_test.html). Note that this won't interact with pre-commit. It will simply
24+
run `Rscript path/to/script.R` (whereas with pre-commit, a {renv} will be activated before running the script).
25+
26+
Also, there are [tests](https://github.com/lorenzwalthert/precommit/blob/main/.github/workflows/end-to-end.yml) to ensure that hooks are correctly registered in `.pre-commit-hooks.yaml`, which you have to adapt if you add a hook.
27+
28+
You can also test them with `pre-commit try-repo` as described in the [documentation](https://pre-commit.com/#pre-commit-try-repo).
29+
3530

3631
# Summary
3732

38-
- add your script to in `inst/bin` and make it executable. Only R scripts are
39-
currently supported as testing is done with `Rscript ...`.
33+
- add your R (with extension) script in `inst/hooks/exported` and make it executable. See other scripts in `inst/hooks/exported` for a starting point for setting up your script.
4034

4135
- register hook in `.pre-commit-hooks.yaml`.
4236

43-
- add two unit tests, test manually with `pre-commit try-repo`.
37+
- add two unit tests, test manually with `pre-commit try-repo` and adapt the [end-to-end test](https://github.com/lorenzwalthert/precommit/blob/main/.github/workflows/end-to-end.yml).
4438

45-
- add a description of the new hook to the `README.Rmd`. Both description and
39+
- add a description of the new hook to `vignettes/available-hooks.Rmd`. Both description and
4640
`yaml` example code.

0 commit comments

Comments
 (0)