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 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.* diff --git a/vignettes/programming_strategy.Rmd b/vignettes/programming_strategy.Rmd index 1590892d..a4e1847d 100644 --- a/vignettes/programming_strategy.Rmd +++ b/vignettes/programming_strategy.Rmd @@ -559,25 +559,35 @@ 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. +`{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 codebase in the coming days. -- **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 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_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:** In the next release an error will be thrown using -`deprecate_stop()`. +- **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 +to change to in their code. -- **Phase 3:** Finally in the 3rd release thereafter the function will be -removed from the package altogether. +- **Phase 3:** After _one year_ and in the closest next release, an error will be thrown +when using the function or argument using `deprecate_stop()` and follow similar process +for Phase 1 and Phase 2. -Information about deprecation timelines must be added to the warning/error message. +- **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 that the deprecation cycle time for a function or argument based on our -current release schedule is 6 months. +**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 @@ -588,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 @@ -609,25 +619,38 @@ 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 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 call to `deprecate_inform()` must be replaced with a call to `deprecate_warn()`. + ```r fun_xxx <- function(dataset, some_param, other_param) { deprecate_warn("x.y.z", "fun_xxx()", "new_fun_xxx()") @@ -639,7 +662,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 @@ -648,21 +671,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. @@ -679,9 +709,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 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