Skip to content
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

Correct documentation of show_progress (no longer utils::txtProgressBar) #2762

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

MichaelChirico
Copy link
Collaborator

No description provided.

@AshesITR AshesITR merged commit 3614b83 into main Feb 19, 2025
19 checks passed
@AshesITR AshesITR deleted the doc-progress branch February 19, 2025 06:02
#' progress bar _via_ [utils::txtProgressBar()]. The default behavior is to show progress in
#' [interactive()] sessions not running a testthat suite.
#' @param show_progress Logical controlling whether to show linting progress with
#' [cli::cli_progress_along()]. The default behavior is to show progress in [interactive()] sessions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My 2 cents: Including such implementation details in user-facing documentation is not a good idea. The users care about showing or displaying the progress bar, not how we decided to implement it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a fair point, and this PR proves it's easy to get out of sync with the implementation.

OTOH, it could wind up in the API if we want to expose some flexibility to the display.

I'm also not familiar with {cli}, but there could be some options() that affect the display -- knowing which {cli} function is used can tell them which options are relevant.

So I'm a bit indifferent -- if you're motivated to change, I'll approve the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants