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

Cli tweaks #1959

Merged
merged 13 commits into from Mar 7, 2024
Merged

Cli tweaks #1959

merged 13 commits into from Mar 7, 2024

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Mar 6, 2024

Follow-up to #1956 with mostly minor tweaks, but 3 more notable changes

  • use_logo() now adds a link to README if it exists. a5ca3b8
  • check_is_package() adds a link to help is whos_asking is provided. 269d2a2

Copy link
Member

@jennybc jennybc left a comment

Choose a reason for hiding this comment

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

Thanks for (most of) this 😅

It would make things easier for me to review and merge if the PR was less of a grab bag. Because some of it's no-brainer stuff I'm definitely looking for. But other changes are more opinions and seem to be where we have different opinions.

R/pr.R Outdated Show resolved Hide resolved
R/pr.R Outdated
Comment on lines 254 to 256
code <- glue('pr_init("{branch}")')
ui_abort(c(
"x" = "No branch named {.val {branch}} exists.",
"_" = "Call {.code {code}} to create a new PR branch."
"_" = "Call {.run usethis::pr_init('{branch}')} to create a new PR branch."
Copy link
Member

Choose a reason for hiding this comment

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

What's the reasoning for fiddling with this?

Copy link
Contributor Author

@olivroy olivroy Mar 6, 2024

Choose a reason for hiding this comment

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

To create a run link. I tweaked it slightly in 2a93251, which seems less invasive?

R/proj.R Outdated Show resolved Hide resolved
R/proj.R Outdated Show resolved Hide resolved
R/rprofile.R Outdated Show resolved Hide resolved
R/test.R Outdated Show resolved Hide resolved
R/tidyverse.R Outdated Show resolved Hide resolved
tests/testthat/_snaps/github-actions.md Outdated Show resolved Hide resolved
tests/testthat/test-proj.R Outdated Show resolved Hide resolved
use_readme_md()
img <- magick::image_write(magick::image_read("logo:"), "logo.png")
withr::local_options(usethis.quiet = FALSE)
expect_snapshot(use_logo("logo.png"), transform = scrub_testpkg)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to actually test for "clickable path"? Because it doesn't feel perceivable from the current snapshot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well the single quotes in the snapshot indicates that some markup was applied (and that it could actually find README.md) 'README.md'.

@jennybc
Copy link
Member

jennybc commented Mar 7, 2024

Thanks!

@jennybc jennybc merged commit bc31759 into r-lib:main Mar 7, 2024
12 of 13 checks passed
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.

None yet

2 participants