Skip to content

Commit ad595b3

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

File tree

1 file changed

+33
-7
lines changed

1 file changed

+33
-7
lines changed

.github/CONTRIBUTING.md

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line 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)