-
Notifications
You must be signed in to change notification settings - Fork 155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
avoid using 'writef' for warning / error style output #1610
Conversation
@@ -89,13 +89,10 @@ renv_available_packages_query <- function(type, repos, quiet = FALSE) { | |||
if (empty(errors)) | |||
return(dbs) | |||
|
|||
if (!is_testing()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if renv_pretty_print
needs a new name, and if it should just write output by default. We almost always use it for "warning"-style output.
renv_scope_options(renv.verbose = TRUE) | ||
writef(lines, renv_difftime_format(elapsed)) | ||
# force output in this scope | ||
renv_scope_caution(TRUE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, but it's forced TRUE here because we want to display this message even if caution was disabled (e.g. as we do in the "synchronized" check)
return(FALSE) | ||
} | ||
|
||
# otherwise, use status to detect if we're synchronized | ||
info <- local({ | ||
renv_scope_options(renv.verbose = FALSE) | ||
renv_scope_caution(FALSE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this should be set fo FALSE
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because we want to avoid the regular "noisy" output from status in this scope -- instead, we want to ask users to run renv::status()
themselves to see the full log.
Addresses #1607.
Addresses #1608.
Still needs tests.
Still needs NEWS.