Skip to content

Commit 68681a2

Browse files
committed
docs: Review contributing guide
1 parent f8af500 commit 68681a2

File tree

1 file changed

+34
-8
lines changed

1 file changed

+34
-8
lines changed

.github/CONTRIBUTING.md

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ See our guide on [how to create a great issue](https://code-review.tidyverse.org
2020

2121
* Fork the package and clone onto your computer. If you haven't done this before, we recommend using `usethis::create_from_github("tidyverse/duckplyr", fork = TRUE)`.
2222

23-
* Install all development dependencies with `pak::pak()`, and then make sure the package passes R CMD check by running `devtools::check()`.
23+
* Install all development dependencies with `pak::pak(dependencies = list("Depends", "Imports", "Suggests", "Config/Needs/development"))`, and then make sure the package passes `R CMD check` by running `devtools::check()`.
2424
If R CMD check doesn't pass cleanly, it's a good idea to ask for help before continuing.
2525

2626
* Create a Git branch for your pull request (PR). We recommend using `usethis::pr_init("brief-description-of-change")`.
@@ -47,18 +47,44 @@ For all functions used in dplyr verbs, translations must be provided.
4747
The code lives in `translate.R` .
4848
New translations must change code in two places:
4949

50-
1. The `switch()` in `rel_find_call()` needs a new entry, together with the package that is home to the function. The top 60 functions, ranked by importance, are already part of that `switch()`, as a comment if they are not implemented yet.
51-
1. The actual translation must be implemented in `rel_translate_lang()`. This is easy for some functions that have similar functions that are already translated, but harder for others. This part of the code is not very clear yet, in particular, argument matching by name is only available for a few functions but should be generalized.
52-
1. Test your implementation in the console with code of the form:
50+
1. The `switch()` in `rel_find_call()` needs a new entry, paired with the name of the package that is home to the function.
51+
The top 60 functions, ranked by importance, are already part of that `switch()`, as a comment if they are not implemented yet.
52+
Example: For adding `lubridate::month()`, add a line of the following form to the `switch()`:
5353

5454
```r
55-
rel_translate(quo(a + 1), data.frame(a = 1)) |>
55+
"month" = "lubridate",
56+
```
57+
58+
1. The actual translation must be implemented in `rel_translate_lang()`.
59+
This is easy for some functions, in particular if similar functions are already translated, but harder for others.
60+
This part of the code is not very clear yet, in particular, argument matching by name is only available for a few functions but should be generalized.
61+
62+
- In some cases (like with `lubridate::month()`), a function of the exact same name already exists in DuckDB, and there's nothing more to do.
63+
64+
- In other cases, a macro must be defined in `relational-duckdb.R` that implements the translation.
65+
66+
- Do you need to do even more work? Let's discuss!
67+
68+
2. Test your implementation in the console with code of the form:
69+
70+
```r
71+
rel_translate(quo(lubridate::month(a)), data.frame(a = Sys.Date())) |>
5672
constructive::construct()
5773
```
5874

59-
1. Add a test for the new translation to the `mutate =` section of `test_extra_arg_map` in `00-funs.R`. (At some point we want to have more specific tests for the translations, for now, this is what it is.)
60-
1. Run `03-tests.R`, commit the changes to the generated code to version control.
61-
1. Update the list in the `limits.Rmd` vignette.
75+
3. Ensure that your implementation computes what you want it to:
76+
77+
```r
78+
duckdb_tibble(a = Sys.Date(), .prudence = "stingy") |>
79+
mutate(lubridate::month(a))
80+
```
81+
82+
4. Add a test for the new translation to the `mutate =` section of `test_extra_arg_map` in `00-funs.R`.
83+
(At some point we want to have more specific tests for the translations, for now, this is what it is.)
84+
85+
5. Run `03-tests.R`, commit the changes to the generated code to version control.
86+
87+
6. Update the list in the `limits.Rmd` vignette.
6288

6389
## Support more options for verbs
6490

0 commit comments

Comments
 (0)