Option to always warn#128
Conversation
Fixes #124
| with = NULL, | ||
| details = NULL, | ||
| id = NULL, | ||
| always = FALSE, |
There was a problem hiding this comment.
Should we make this frequency = c("always", "regularly", "once") as in rlang::warn()?
There was a problem hiding this comment.
I don't think so — that feels too much like tying this interface to its implementation. (Not that lifecycle currently uses frequency but we could now refactor to do so).
I think the goal here is to create a hierarchy of annoyingness that we can gradually move functions along over time. I could imagine something more like annoy = c("a little", "a lot") but we'd need to think about that more, and adding one extra step seems like enough for now.
@DavisVaughan any thoughts?
There was a problem hiding this comment.
I'm fine with it being a boolean because I do think we just need exactly 1 more step in the process:
soft (in tests) -> warn (8 hours) -> warn (always) -> error
| if (always || needs_warning(id)) { | ||
| # Prevent warning from being displayed again | ||
| env_poke(deprecation_env, id, Sys.time()) | ||
|
|
There was a problem hiding this comment.
The warning message that gets thrown when always = TRUE isn't exactly right, right? i.e. we don't want the footer that says it is displayed once every 8 hours
There was a problem hiding this comment.
Maybe add a new snapshot test that captures the tweaked message to ensure no footer is added when always = TRUE
There was a problem hiding this comment.
Probably best to refactor and use rlang here?
There was a problem hiding this comment.
I'll do a little refactoring of the message generation, but I think refactoring to use new rlang features is out of scope. I had a go at it yesterday but the lifecycle warnings are bit too custom for me to easily see how to update.
| with = NULL, | ||
| details = NULL, | ||
| id = NULL, | ||
| always = FALSE, |
There was a problem hiding this comment.
I'm fine with it being a boolean because I do think we just need exactly 1 more step in the process:
soft (in tests) -> warn (8 hours) -> warn (always) -> error
Conflicts: NEWS.md
And tweak how footer is added to message
3 participants
Fixes #124