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.

Sign up for GitHub

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

Jump to bottom

Provide way to register extra packages for revdep checks #1745

Merged
merged 6 commits into from
Jan 24, 2023
Merged

Provide way to register extra packages for revdep checks #1745

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7a608f5
Provide way to register extra packages for revdep checks
hadley Jan 19, 2023
e795b11
Apply suggestions from code review
hadley Jan 19, 2023
04e64e8
Re-document
hadley Jan 19, 2023
51ecba1
Apply suggestions from code review
hadley Jan 20, 2023
6df0065
Merge commit '26a843c148ded63300f3d17f6fba899611640aac'
hadley Jan 20, 2023
94d0d3d
Merge branch 'main' into extra-revdeps
hadley Jan 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# usethis (development version)

* `use_release_issue()` now uses internal `release_extra_revdeps()` to
add extra revdep sources. Currently only use for internal Posit tooling,
but we hope to extend to all users in the future (#1610).

* `use_data()` now sets the appropriate minimal R version in `DESCRIPTION`,
depending on which serialization format `version` you choose (@dpprdan, #1672).

Expand Down
59 changes: 46 additions & 13 deletions R/release.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,18 @@
#' The checklist contains a generic set of steps that we've found to be helpful,
#' based on the type of release ("patch", "minor", or "major"). You're
#' encouraged to edit the issue to customize this list to meet your needs.
#' If you want to consistently add extra bullets for every release, you can
#' include your own custom bullets by providing a (unexported) a
#' `release_bullets()` function that returns a character vector.
#' (For historical reasons, `release_questions()` is also supported).
#'
#' ## Customization
#'
#' * If you want to consistently add extra bullets for every release, you can
#' include your own custom bullets by providing a (unexported) a
hadley marked this conversation as resolved.
Show resolved Hide resolved
#' `release_bullets()` function that returns a character vector.
#' (For historical reasons, `release_questions()` is also supported).
#'
#' * If you want to check additional packages in the revdep check process
hadley marked this conversation as resolved.
Show resolved Hide resolved
#' provide an (unexported) `release_extra_revdeps()` function that
#' return a character vector. This is currently only supported for
hadley marked this conversation as resolved.
Show resolved Hide resolved
#' Posit internal check tooling.
#'
#' @param version Optional version number for release. If unspecified, you can
#' make an interactive choice.
Expand Down Expand Up @@ -94,13 +102,12 @@ release_checklist <- function(version, on_cran) {
todo("`rhub::check_for_cran()`"),
todo("`rhub::check(platform = 'ubuntu-rchk')`", has_src),
todo("`rhub::check_with_sanitizers()`", has_src),
todo("`revdepcheck::revdep_check(num_workers = 4)`", on_cran && !is_rstudio_pkg),
todo("`revdepcheck::cloud_check()`", on_cran && is_rstudio_pkg),
release_revdepcheck(on_cran, is_rstudio_pkg),
todo("Update `cran-comments.md`", on_cran),
todo("`git push`"),
todo("Draft blog post", type != "patch"),
todo("Slack link to draft blog in #open-source-comms", type != "patch" && is_rstudio_pkg),
release_extra(),
release_extra_bullets(),
"",
"Submit to CRAN:",
"",
Expand All @@ -122,13 +129,32 @@ release_checklist <- function(version, on_cran) {
)
}

release_extra <- function(env = NULL) {
if (is.null(env)) {
env <- tryCatch(
pkg_env(project_name()),
error = function(e) emptyenv()
)
release_revdepcheck <- function(on_cran = TRUE, is_rstudio_pkg = TRUE, env = NULL) {
if (!on_cran) {
return()
}

env <- env %||% safe_pkg_env()
if (env_has(env, "release_extra_revdeps")) {
extra <- env$release_extra_revdeps()
hadley marked this conversation as resolved.
Show resolved Hide resolved
} else {
extra <- character()
}

if (is_rstudio_pkg) {
if (length(extra) > 1) {
hadley marked this conversation as resolved.
Show resolved Hide resolved
extra_code <- paste0(deparse(extra), collapse = "")
Copy link
Member

Choose a reason for hiding this comment

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

I had to run this locally to figure out what it did 😅

Copy link
Member

Choose a reason for hiding this comment

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

Can it just be deparse(extra)?

extra <- letters[1:3]
identical(deparse(extra), paste0(deparse(extra), collapse = ""))
#> [1] TRUE

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably would be fine here, but it's safer to always collapse into a single string:

extra <- paste0("longpackagename", letters[1:10])
length(deparse(extra))
#> [1] 3

Created on 2023-01-24 with reprex v2.0.2

todo("`revdepcheck::cloud_check(extra_revdeps = {extra_code})`")
} else {
todo("`revdepcheck::cloud_check()`")
}
} else {
todo("`revdepcheck::revdep_check(num_workers = 4)`")
}
}

release_extra_bullets <- function(env = NULL) {
env <- env %||% safe_pkg_env()

if (env_has(env, "release_bullets")) {
paste0("* [ ] ", env$release_bullets())
Expand All @@ -140,6 +166,13 @@ release_extra <- function(env = NULL) {
}
}

safe_pkg_env <- function() {
tryCatch(
pkg_env(project_name()),
hadley marked this conversation as resolved.
Show resolved Hide resolved
error = function(e) emptyenv()
)
}

release_type <- function(version) {
x <- unclass(numeric_version(version))[[1]]
n <- length(x)
Expand Down
2 changes: 1 addition & 1 deletion man/use_github_labels.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion man/use_release_issue.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 19 additions & 0 deletions tests/testthat/_snaps/release.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,25 @@
* [ ] Tweet
* [ ] Add link to blog post in pkgdown news menu

# construct correct revdep bullet

Code
release_revdepcheck(on_cran = FALSE)
Output
NULL
Code
release_revdepcheck(on_cran = TRUE, is_rstudio_pkg = FALSE)
Output
[1] "* [ ] `revdepcheck::revdep_check(num_workers = 4)`"
Code
release_revdepcheck(on_cran = TRUE, is_rstudio_pkg = TRUE)
Output
[1] "* [ ] `revdepcheck::cloud_check()`"
Code
release_revdepcheck(on_cran = TRUE, is_rstudio_pkg = TRUE, env = env)
Output
[1] "* [ ] `revdepcheck::cloud_check(extra_revdeps = c(\"waldo\", \"testthat\"))`"

# RStudio-ness detection works

Code
Expand Down
18 changes: 15 additions & 3 deletions tests/testthat/test-release.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,25 @@ test_that("release bullets don't change accidentally", {

test_that("get extra news bullets if available", {
env <- env(release_bullets = function() "Extra bullets")
expect_equal(release_extra(env), "* [ ] Extra bullets")
expect_equal(release_extra_bullets(env), "* [ ] Extra bullets")

env <- env(release_questions = function() "Extra bullets")
expect_equal(release_extra(env), "* [ ] Extra bullets")
expect_equal(release_extra_bullets(env), "* [ ] Extra bullets")

env <- env()
expect_equal(release_extra(env), character())
expect_equal(release_extra_bullets(env), character())
})

test_that("construct correct revdep bullet", {
create_local_package()
env <- env(release_extra_revdeps = function() c("waldo", "testthat"))
Copy link
Member

Choose a reason for hiding this comment

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

I assume this would have failed to give the right answer if it returned 1 character string instead of 2 because of that length() > 1 bug 😬


expect_snapshot({
release_revdepcheck(on_cran = FALSE)
release_revdepcheck(on_cran = TRUE, is_rstudio_pkg = FALSE)
release_revdepcheck(on_cran = TRUE, is_rstudio_pkg = TRUE)
release_revdepcheck(on_cran = TRUE, is_rstudio_pkg = TRUE, env = env)
})
})

test_that("RStudio-ness detection works", {
Expand Down