From ef2f6c545bf7b68fbbda8a3033a9891f47a1faaa Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Mon, 7 Oct 2024 01:40:10 +0000 Subject: [PATCH 1/9] docs: #466 deprecate 2.0 --- vignettes/programming_strategy.Rmd | 45 +++++++++++++++++------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index 1590892d..487d5683 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -559,25 +559,32 @@ See [Writing Unit Tests in {admiral}](unit_test_guidance.html#writing-unit-tests # Deprecation -As `{admiral}` is still evolving, functions or arguments may need to be removed -or replaced with more efficient options from one release to another. In such -cases, the relevant function or argument must be marked as deprecated. This -deprecation is done in three phases over our release cycles. - -- **Phase 1:** In the release where the identified function or argument is to -be deprecated there will be a warning issued when using the function or argument -using `deprecate_warn()`. - -- **Phase 2:** In the next release an error will be thrown using -`deprecate_stop()`. - -- **Phase 3:** Finally in the 3rd release thereafter the function will be -removed from the package altogether. - -Information about deprecation timelines must be added to the warning/error message. - -Note that the deprecation cycle time for a function or argument based on our -current release schedule is 6 months. +`{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. + +- **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 +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 +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 +to change to in their code. + +- **Phase 3:** After _one year_ and in the closest next release, an error will be thrown +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 +function or argument will be completely removed from `{admiral}`. + +**Note:** 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! ## Documentation From 5c00f05bd87fff0d32ccf16e55d1d9085dd85540 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Tue, 8 Oct 2024 19:29:50 +0000 Subject: [PATCH 2/9] 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 From a78fc84fe2086f940e0e2e42a1406f93999167db Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Tue, 8 Oct 2024 19:31:49 +0000 Subject: [PATCH 3/9] chore: #466 news --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index be4cbab7..f469c9e1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -17,6 +17,7 @@ - Removed at v1.0.0 `assert_function_param()` ## Documentation + - Deprecation Strategy updated for the long haul! (#466) ## Other From ead0e130e7f04488956db702254a788e0bbe6c38 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Tue, 8 Oct 2024 19:34:57 +0000 Subject: [PATCH 4/9] chore: #466 pkgdown yml --- _pkgdown.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/_pkgdown.yml b/_pkgdown.yml index 7d62bcf6..30d3cd76 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -91,11 +91,12 @@ reference: - title: Deprecated desc: | - As `{admiral}` is still evolving, functions/arguments may need to be removed or replaced over time. In such cases, the function/argument will enter the following 6-month deprecation cycle: + As `{admiral}` is still evolving, functions/arguments may need to be removed or replaced over time. In such cases, the function/argument will enter a 3 year deprecation cycle: - * In the first release (0-3 months), there will be a warning issued if you use the function/argument, but it will still be available to use. - * In the following release (3-6 months), an error will be produced if you use the function/argument. - * Finally, from the 3rd release (6 months) onwards, the function/argument will be removed from `{admiral}` and its documentation completely. + * In the first release (1 year), there will be a message issued if you use the function/argument, but it will still be available to use. + * In the following release (1 year), an warning will be produced if you use the function/argument. + * In the following release (1 year), an error will be produced if you use the function/argument. + * Finally, after 3 years onwards, the function/argument will be removed from `{admiral}` and its documentation completely. *Note: Guidance on replacement functionality can be found in the warning/error message produced or in the function's documentation.* From 823ff6c799eedbed9ab76adc9819c79ca3b92e5a Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 9 Oct 2024 10:31:20 -0400 Subject: [PATCH 5/9] Update vignettes/programming_strategy.Rmd Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com> --- vignettes/programming_strategy.Rmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index 4ebf3dd6..9d0ea1df 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -576,7 +576,7 @@ warning message will run for and a recommendation on which function or argument to change to in their code. - **Phase 3:** After _one year_ and in the closest next release, an error will be thrown -using the function or argument using `deprecate_stop()` and follow similar process +when using the function or argument using `deprecate_stop()` and follow similar process for Phase 1 and Phase 2. - **Phase 4:** Finally after three years from the time of being identified for deprecation, the From 56eec0364474b348bf198b1395633ff4598c077b Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 9 Oct 2024 10:38:14 -0400 Subject: [PATCH 6/9] Update vignettes/programming_strategy.Rmd Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com> --- vignettes/programming_strategy.Rmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index 9d0ea1df..34d5e9c4 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -586,7 +586,7 @@ function or argument will be completely removed from `{admiral}`. 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 +**NB:** Take care with `NEWS.md` entries for deprecation as the person continuing this process might not be you! ## Documentation From 76e4a131f7cbf009ded2e69400d8ea8e126d594c Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 9 Oct 2024 10:38:29 -0400 Subject: [PATCH 7/9] Update vignettes/programming_strategy.Rmd Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com> --- vignettes/programming_strategy.Rmd | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index 34d5e9c4..3532c821 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -649,8 +649,7 @@ fun_xxx <- function(dataset, some_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. +the call to `deprecate_inform()` must be replaced with a call to `deprecate_warn()`. ```r fun_xxx <- function(dataset, some_param, other_param) { From c9304fdcab69b4a424cf663927f8d70981371453 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 9 Oct 2024 10:39:09 -0400 Subject: [PATCH 8/9] Update vignettes/programming_strategy.Rmd Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com> --- vignettes/programming_strategy.Rmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index 3532c821..b4a0ea57 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -724,7 +724,7 @@ arguments which are not passed as `exprs()` argument (e.g. `new_var = VAR1` or ## 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. +file [^1] of the function to ensure that a message, 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 From f0d67efeb17d094972a06c12b5513f67789f924f Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 9 Oct 2024 10:39:19 -0400 Subject: [PATCH 9/9] Update vignettes/programming_strategy.Rmd Co-authored-by: Stefan Bundfuss <80953585+bundfussr@users.noreply.github.com> --- vignettes/programming_strategy.Rmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index b4a0ea57..a4e1847d 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -619,7 +619,7 @@ The documentation will be updated at: + the `@param` level for a argument. ``` - @param old_param *Deprecated*, please use `new_param` instead. + @param old_param `r lifecycle::badge("deprecated")` Please use `new_param` instead. ``` ## Handling of Messages, Warnings and Errors