Warn once per session, not once every 8 hours within a session

NEWS.md

@@ -4,7 +4,7 @@
 
 * `deprecate_soft()` and `deprecate_warn()` are faster thanks to some internal refactoring.
 
-* `deprecate_soft()` now actually warns after every 8 hours.
+* `deprecate_soft()` and `deprecate_warn()` now only warn once per session rather than attempting to warn once every 8 hours. This never actually worked.
 
 * Improvements to `lint_lifecycle()` and `lint_tidyverse_lifecycle()` (@AshesITR):
   * Updated to support lintr >= 3.0.0 (#178).

R/deprecate.R

@@ -14,7 +14,7 @@
 #'
 #' * `deprecate_stop()` fails unconditionally.
 #'
-#' Warnings are only issued once every 8 hours to avoid overwhelming
+#' Warnings are only issued once per session to avoid overwhelming
 #' the user. Control with [`options(lifecycle_verbosity)`][verbosity].
 #'
 #' @section Conditions:
@@ -139,9 +139,9 @@ deprecate_soft <- function(
 }
 
 #' @rdname deprecate_soft
-#' @param always If `FALSE`, the default, will warn every 8 hours.  If
+#' @param always If `FALSE`, the default, will warn once per session. If
 #'   `TRUE`, will always warn in direct usages. Indirect usages keep
-#'   warning every 8 hours to avoid disrupting users who can't fix the
+#'   warning once per session to avoid disrupting users who can't fix the
 #'   issue. Only use `always = TRUE` after at least one release with
 #'   the default.
 #' @export
@@ -229,8 +229,8 @@ deprecate_warn0 <- function(
     return()
   }
 
-  # Prevent warning from being displayed again
-  env_poke(deprecation_env, id, Sys.time())
+  # Prevent warning from being displayed again this session
+  env_poke(deprecation_env, id, TRUE)
 
   footer <- function(...) {
     footer <- NULL
@@ -266,7 +266,7 @@ deprecate_warn0 <- function(
     if (is_interactive()) {
       footer <- c(
         footer,
-        if (!always) silver("This warning is displayed once every 8 hours."),
+        if (!always) silver("This warning is displayed once per session."),
         silver(
           "Call `lifecycle::last_lifecycle_warnings()` to see where this warning was generated."
         )
@@ -458,21 +458,5 @@ from_testthat <- function(env) {
 
 needs_warning <- function(id, call = caller_env()) {
   check_string(id, call = call)
-
-  last <- deprecation_env[[id]]
-  if (is_null(last)) {
-    return(TRUE)
-  }
-
-  if (!inherits(last, "POSIXct")) {
-    abort(
-      "Expected `POSIXct` value in `needs_warning()`.",
-      .internal = TRUE
-    )
-  }
-
-  # Warn every 8 hours
-  # Must unclass to ensure we always compare in units of seconds, also much
-  # faster this way.
-  (unclass(Sys.time()) - unclass(last)) > (8 * 60 * 60)
+  is_null(deprecation_env[[id]])
 }

R/expect.R

@@ -25,7 +25,7 @@
 #' @details
 #' `expect_deprecated()` sets the [lifecycle_verbosity][verbosity]
 #' option to `"warning"` to enforce deprecation warnings which are
-#' otherwise only shown once every 8 hours.
+#' otherwise only shown once per session.
 #'
 #' @export
 expect_deprecated <- function(expr, regexp = NULL, ...) {

R/verbosity.R

@@ -14,7 +14,7 @@
 #' `lifecycle_verbosity`. It can be set to:
 #'
 #' * `"quiet"` to suppress all deprecation messages.
-#' * `"default"` or `NULL` to warn once every 8 hours.
+#' * `"default"` or `NULL` to warn once per session.
 #' * `"warning"` to warn every time.
 #' * `"error"` to error instead of warning.
 #'

tests/testthat/_snaps/deprecate.md

@@ -194,9 +194,3 @@
       Error:
       ! `id` must be a single string, not a number.
 
----
-
-    Expected `POSIXct` value in `needs_warning()`.
-    i This is an internal error that was detected in the lifecycle package.
-      Please report it at <https://github.com/r-lib/lifecycle/issues> with a reprex (<https://tidyverse.org/help/>) and the full backtrace.
-

tests/testthat/test-deprecate.R

@@ -228,13 +228,6 @@ test_that("needs_warning works as expected", {
   expect_snapshot(needs_warning(1), error = TRUE)
   expect_true(needs_warning("test"))
 
-  env_poke(deprecation_env, "test", Sys.time())
+  env_poke(deprecation_env, "test", TRUE)
   expect_false(needs_warning("test"))
-
-  # More than 8 hours
-  env_poke(deprecation_env, "test", Sys.time() - 9 * 60 * 60)
-  expect_true(needs_warning("test"))
-
-  env_poke(deprecation_env, "test", "x")
-  expect_snapshot_error(needs_warning("test"))
 })

tests/testthat/test-lifecycle.R

@@ -61,20 +61,20 @@ test_that("deprecation warnings are not displayed again", {
   local_interactive()
 
   wrn <- catch_cnd(
-    deprecate_warn("1.0.0", "foo()", id = "once-every-8-hours-note"),
+    deprecate_warn("1.0.0", "foo()", id = "once-per-session-note"),
     classes = "warning"
   )
   footer <- cnd_footer(wrn)
-  expect_true(is_character(footer) && any(grepl("once every 8 hours", footer)))
+  expect_true(is_character(footer) && any(grepl("once per session", footer)))
 
   local_options(lifecycle_verbosity = "warning")
 
   wrn <- catch_cnd(deprecate_warn(
     "1.0.0",
     "foo()",
-    id = "once-every-8-hours-no-note"
+    id = "once-per-session-no-note"
   ))
-  expect_false(grepl("once every 8 hours", conditionMessage(wrn)))
+  expect_false(grepl("once per session", conditionMessage(wrn)))
 })
 
 test_that("the topenv of the empty env is not the global env", {