Skip to content

Commit

Permalink
docs: #466 unit tests and roxygen
Browse files Browse the repository at this point in the history
  • Loading branch information
bms63 committed Oct 8, 2024
1 parent ef2f6c5 commit 5c00f05
Showing 1 changed file with 56 additions and 19 deletions.
75 changes: 56 additions & 19 deletions vignettes/programming_strategy.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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()")
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down

0 comments on commit 5c00f05

Please sign in to comment.