From 5c00f05bd87fff0d32ccf16e55d1d9085dd85540 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Tue, 8 Oct 2024 19:29:50 +0000 Subject: [PATCH] docs: #466 unit tests and roxygen --- vignettes/programming_strategy.Rmd | 75 ++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index 487d5683..4ebf3dd6 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -561,15 +561,15 @@ See [Writing Unit Tests in {admiral}](unit_test_guidance.html#writing-unit-tests `{admiral}` has reached a level of maturity with the release of `1.0.0` in December 2023. The below deprecation strategy provides stability to users while allowing admiral developers -the ability to remove and update the code base in the coming days. +the ability to remove and update the codebase in the coming days. - **Phase 1:** In the release where the identified function or argument is to be deprecated, there will be a message issued when using the function or argument -using `deprecate_nicely()`. This message will appear to the user for _two years_. The +using `deprecate_inform()`. This message will appear to the user for _one year_. The message will include the date this message will run for and a recommendation on which function or argument to change to in their code. -- **Phase 2:** After _two years_ and in the closet next release, a warning will be +- **Phase 2:** After _one year_ and in the closet next release, a warning will be issued when using the function or argument using `deprecate_warn()`. This warning message will appear to the user for _one year_. The message will include the date this warning message will run for and a recommendation on which function or argument @@ -579,13 +579,16 @@ to change to in their code. using the function or argument using `deprecate_stop()` and follow similar process for Phase 1 and Phase 2. -- **Phase 4:** Finally four years from the time of being identified for deprecation, the +- **Phase 4:** Finally after three years from the time of being identified for deprecation, the function or argument will be completely removed from `{admiral}`. -**Note:** Major/Minor release make the most sense for deprecation updates. However, if +**NB:** Major/Minor release make the most sense for deprecation updates. However, if a release cycle becomes multiple years, then patch releases should be considered to help keep `{admiral}` neat and tidy! +**NB:** Take care with `News.md` entries for deprecation as the person continuing this +process might not be you! + ## Documentation If a function or argument is removed, the documentation must be updated to @@ -595,7 +598,7 @@ function/argument should be used instead. The documentation will be updated at: + the description level for a function, -+ the `@keywords` and`@family` roxygen tags will be replaced with `deprecated` ++ the `@keywords` and `@family` roxygen tags will be replaced with `deprecated` ```{r, eval=FALSE} #' Title of the function @@ -619,22 +622,36 @@ The documentation will be updated at: @param old_param *Deprecated*, please use `new_param` instead. ``` -## Handling of Warning and Error +## Handling of Messages, Warnings and Errors When a function or argument is deprecated, the function must be updated to issue -a warning or error using `deprecate_warn()` and `deprecate_stop()`, -respectively, as described above. +a message, warning or error using `deprecate_inform()`, `deprecate_warn()` or +`deprecate_stop()`, respectively, as described above. There should be a test case added in the test file of the function that checks -whether this warning/error is issued as appropriate when using the deprecated +whether this message/warning/error is issued as appropriate when using the deprecated function or argument. ### Function -In the initial release in which a function is deprecated the original function -body must be replaced with a call to `deprecate_warn()` and subsequently all +**Phase 1:** In the initial release in which a function is deprecated the original function +body must be replaced with a call to `deprecate_inform()` and subsequently all arguments should be passed on to the new function. +```r +fun_xxx <- function(dataset, some_param, other_param) { + deprecate_inform("x.y.z", "fun_xxx()", "new_fun_xxx()") + new_fun_xxx( + dataset = dataset, + some_param = some_param, + other_param = other_param + ) +} +``` +**Phase 2:** In the next year/closest release in which a function is deprecated +the original function body must be replaced with a call to `deprecate_warn()` and +subsequently all arguments should be passed on to the new function. + ```r fun_xxx <- function(dataset, some_param, other_param) { deprecate_warn("x.y.z", "fun_xxx()", "new_fun_xxx()") @@ -646,7 +663,7 @@ fun_xxx <- function(dataset, some_param, other_param) { } ``` -In the following release the function body should be changed to just include a call +**Phase 3:** In the following release the function body should be changed to just include a call to `deprecate_stop()`. ```r @@ -655,21 +672,28 @@ fun_xxx <- function(dataset, some_param, other_param) { } ``` -Finally, in the next release the function should be removed from the package. +**Phase 4:** Finally, in the next release the function should be removed from the package. ### Argument -If an argument is removed and is not replaced, an **error** must be generated: +**Phase 1:** If the argument is renamed or replaced, a **message** must be issued and the +new argument takes the value of the old argument until the next release. Note: +arguments which are not passed as `exprs()` argument (e.g. `new_var = VAR1` or +`filter = AVAL >10`) will need to be quoted. -``` +``` ### BEGIN DEPRECATION if (!missing(old_param)) { - deprecate_stop("x.y.z", "fun_xxx(old_param = )", "fun_xxx(new_param = )") + deprecate_inform("x.y.z", "fun_xxx(old_param = )", "fun_xxx(new_param = )") + # old_param is given using exprs() + new_param <- old_param + # old_param is NOT given using exprs() + new_param <- enexpr(old_param) } ### END DEPRECATION ``` -If the argument is renamed or replaced, a **warning** must be issued and the +**Phase 2:** If the argument is renamed or replaced, a **warning** must be issued and the new argument takes the value of the old argument until the next release. Note: arguments which are not passed as `exprs()` argument (e.g. `new_var = VAR1` or `filter = AVAL >10`) will need to be quoted. @@ -686,9 +710,22 @@ arguments which are not passed as `exprs()` argument (e.g. `new_var = VAR1` or ### END DEPRECATION ``` +**Phase 3:** If an argument is removed and is not replaced, an **error** must be generated: + +``` +### BEGIN DEPRECATION + if (!missing(old_param)) { + deprecate_stop("x.y.z", "fun_xxx(old_param = )", "fun_xxx(new_param = )") + } +### END DEPRECATION +``` + + + ## Unit Testing -Unit tests for deprecated functions and arguments must be added to the test file [^1] of the function to ensure that a warning or error is issued. +Unit tests for deprecated functions and arguments must be added to the test +file [^1] of the function to ensure that a warning or error is issued. [^1]: For example, if `derive_var_example()` is going to be deprecated and it is defined in `examples.R`, the unit tests are in