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

More upkeep polishing #1909

Merged
merged 5 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 0 additions & 3 deletions R/release.R
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ release_checklist <- function(version, on_cran) {
has_readme <- file_exists(proj_path("README.Rmd"))
has_github_links <- has_github_links()
is_posit_pkg <- is_posit_pkg()
tidy_min_r_version <- tidy_minimum_r_version()

milestone_num <- NA # for testing (and general fallback)
if (uses_git() && curl::has_internet()) {
Expand Down Expand Up @@ -112,8 +111,6 @@ release_checklist <- function(version, on_cran) {
Check if any deprecation processes should be advanced, as described in \\
[Gradual deprecation](https://lifecycle.r-lib.org/articles/communicate.html#gradual-deprecation)",
type != "patch" && has_lifecycle),
todo("Bump required R version in DESCRIPTION to {tidy_min_r_version}",
Copy link
Member Author

Choose a reason for hiding this comment

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

Now in upkeep, rather than release.

Copy link
Member

Choose a reason for hiding this comment

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

I kinda like it as part of the release checklist, but not a hill I will die on.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have strong feelings either except that it should be in one or the other.

is_posit_pkg && tidy_min_r_version > pkg_minimum_r_version()),
todo("`usethis::use_news_md()`", on_cran && !has_news),
todo("[Polish NEWS](https://style.tidyverse.org/news.html#news-release)", on_cran),
todo("`usethis::use_github_links()`", !has_github_links),
Expand Down
34 changes: 25 additions & 9 deletions R/upkeep.R
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,22 @@ use_tidy_upkeep_issue <- function(year = NULL) {
make_upkeep_issue(year = year, tidy = TRUE)
}

tidy_upkeep_checklist <- function(year = NULL,
posit_pkg = is_posit_pkg(),
posit_person_ok = is_posit_person_canonical(),
repo_spec = "OWNER/REPO") {
# for mocking
Sys.Date <- NULL

tidy_upkeep_checklist <- function(year = NULL, repo_spec = "OWNER/REPO") {

posit_pkg <- is_posit_pkg()
posit_person_ok <- is_posit_person_canonical()

year <- year %||% 2000

bullets <- c()
bullets <- c(
"### To begin",
"",
todo('`pr_init("upkeep-{format(Sys.Date(), "%Y-%m")}")`'),
""
)

if (year <= 2000) {
bullets <- c(
Expand Down Expand Up @@ -202,7 +211,7 @@ tidy_upkeep_checklist <- function(year = NULL,
with DESCRIPTION changes",
author_has_rstudio_email() || (posit_pkg && !posit_person_ok)
),
todo("`usethis::use_tidy_logo()`"),
todo("`usethis::use_tidy_logo(); pkgdown::build_favicons(overwrite = TRUE)`"),
todo("`usethis::use_tidy_coc()`"),
todo(
"Modernize citation files; see updated `use_citation()`",
Expand Down Expand Up @@ -232,10 +241,13 @@ tidy_upkeep_checklist <- function(year = NULL,

bullets <- c(
bullets,
"### Eternal",
"### To finish",
"",
todo("`usethis::use_mit_license()`", grepl("MIT", desc$get_field("License"))),
todo('`usethis::use_package("R", "Depends", "{tidy_minimum_r_version()}")`'),
todo(
'`usethis::use_package("R", "Depends", "{tidy_minimum_r_version()}")`',
tidy_minimum_r_version() > pkg_minimum_r_version()
),
todo("`usethis::use_tidy_description()`"),
todo("`usethis::use_tidy_github_actions()`"),
todo("`devtools::build_readme()`"),
Expand Down Expand Up @@ -299,10 +311,14 @@ checklist_footer <- function(tidy) {
tidy_fun <- if (tidy) "tidy_" else ""
glue('<sup>\\
Created on {Sys.Date()} with `usethis::use_{tidy_fun}upkeep_issue()`, using \\
[usethis v{utils::packageVersion("usethis")}](https://usethis.r-lib.org)\\
[usethis v{usethis_version()}](https://usethis.r-lib.org)\\
</sup>')
}

usethis_version <- function() {
utils::packageVersion("usethis")
}

has_old_cran_comments <- function() {
cc <- proj_path("cran-comments.md")
file_exists(cc) &&
Expand Down
1 change: 0 additions & 1 deletion tests/testthat/_snaps/release.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@
* [ ] `git pull`
* [ ] Check [current CRAN check results](https://cran.rstudio.org/web/checks/check_results_{TESTPKG}.html)
* [ ] Bump required R version in DESCRIPTION to 3.6
* [ ] `usethis::use_news_md()`
* [ ] [Polish NEWS](https://style.tidyverse.org/news.html#news-release)
* [ ] `usethis::use_github_links()`
Expand Down
16 changes: 10 additions & 6 deletions tests/testthat/_snaps/upkeep.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
# tidy upkeep bullets don't change accidentally

Code
writeLines(tidy_upkeep_checklist(posit_pkg = TRUE, posit_person_ok = FALSE))
writeLines(tidy_upkeep_checklist())
Output
### To begin

* [ ] `pr_init("upkeep-2023-01")`

### Pre-history

* [ ] `usethis::use_readme_rmd()`
Expand Down Expand Up @@ -36,7 +40,7 @@
* [ ] Update email addresses *@rstudio.com -> *@posit.co
* [ ] Update copyright holder in DESCRIPTION: `person("Posit Software, PBC", role = c("cph", "fnd"))`
* [ ] Run `devtools::document()` to re-generate package-level help topic with DESCRIPTION changes
* [ ] `usethis::use_tidy_logo()`
* [ ] `usethis::use_tidy_logo(); pkgdown::build_favicons(overwrite = TRUE)`
* [ ] `usethis::use_tidy_coc()`
* [ ] Use `pak::pak("OWNER/REPO")` in README
* [ ] Consider running `usethis::use_tidy_dependencies()` and/or replace compat files with `use_standalone()`
Expand All @@ -45,7 +49,7 @@
or [file an issue](new) if you don't have time to do it now
* [ ] Add alt-text to pictures, plots, etc; see https://posit.co/blog/knitr-fig-alt/ for examples

### Eternal
### To finish

* [ ] `usethis::use_mit_license()`
* [ ] `usethis::use_package("R", "Depends", "3.6")`
Expand All @@ -54,7 +58,7 @@
* [ ] `devtools::build_readme()`
* [ ] [Re-publish released site](https://pkgdown.r-lib.org/dev/articles/how-to-update-released-site.html) if needed

<sup>Created on DATE with `usethis::use_tidy_upkeep_issue()`, using [usethis vVERSION](https://usethis.r-lib.org)</sup>
<sup>Created on 2023-01-01 with `usethis::use_tidy_upkeep_issue()`, using [usethis v1.1.0](https://usethis.r-lib.org)</sup>

# upkeep bullets don't change accidentally

Expand All @@ -76,7 +80,7 @@
Updating workflows to the latest version will often fix troublesome actions:
* [ ] `usethis::use_github_action('check-standard')`

<sup>Created on DATE with `usethis::use_upkeep_issue()`, using [usethis vVERSION](https://usethis.r-lib.org)</sup>
<sup>Created on 2023-01-01 with `usethis::use_upkeep_issue()`, using [usethis v1.1.0](https://usethis.r-lib.org)</sup>

---

Expand All @@ -102,5 +106,5 @@
* [ ] `usethis::use_github_action('check-standard')`
* [ ] `usethis::use_github_action('test-coverage')`

<sup>Created on DATE with `usethis::use_upkeep_issue()`, using [usethis vVERSION](https://usethis.r-lib.org)</sup>
<sup>Created on 2023-01-01 with `usethis::use_upkeep_issue()`, using [usethis v1.1.0](https://usethis.r-lib.org)</sup>

5 changes: 0 additions & 5 deletions tests/testthat/helper.R
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,3 @@ test_file <- function(fname) testthat::test_path("ref", fname)

expect_proj_file <- function(...) expect_true(file_exists(proj_path(...)))
expect_proj_dir <- function(...) expect_true(dir_exists(proj_path(...)))

scrub_checklist_footer <- function(text) {
gsub("(^<sup>.+on )[-/0-9]{10}(.+ v)[0-9.]{3,12}(.+</sup>$)",
"\\1DATE\\2VERSION\\3", text)
}
51 changes: 22 additions & 29 deletions tests/testthat/test-upkeep.R
Original file line number Diff line number Diff line change
@@ -1,56 +1,49 @@
test_that("tidy upkeep bullets don't change accidentally", {
withr::local_options(
usethis.description = list(
"Authors@R" = utils::person(
"Jane", "Doe",
email = "jane@rstudio.com",
role = c("aut", "cre")
),
License = "MIT + file LICENSE"
)
)
create_local_package()
use_mit_license()

expect_snapshot(
writeLines(tidy_upkeep_checklist(posit_pkg = TRUE, posit_person_ok = FALSE)),
transform = scrub_checklist_footer
local_mocked_bindings(
Copy link
Member Author

Choose a reason for hiding this comment

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

I probably should've done this in a separate PR, but using mocking for everything makes the tests quite a bit simpler.

Sys.Date = function() as.Date("2023-01-01"),
usethis_version = function() "1.1.0",
author_has_rstudio_email = function() TRUE,
is_posit_pkg = function() TRUE,
is_posit_person_canonical = function() FALSE
)

expect_snapshot(writeLines(tidy_upkeep_checklist()))
})

test_that("upkeep bullets don't change accidentally",{
skip_if_no_git_user()
withr::local_options(usethis.description = NULL)

create_local_package()
local_mocked_bindings(git_default_branch = function() "main")
use_cran_comments()

expect_snapshot(
writeLines(upkeep_checklist()),
transform = scrub_checklist_footer
local_mocked_bindings(
Sys.Date = function() as.Date("2023-01-01"),
usethis_version = function() "1.1.0",
git_default_branch = function() "main"
)

expect_snapshot(writeLines(upkeep_checklist()))

# Add some files to test conditional todos
use_code_of_conduct("jane.doe@foofymail.com")
use_testthat()
withr::local_file("cran-comments.md")
writeLines(
"## Test environments\\n\\n* local Ubuntu\\n\\# R CMD check results\\n",
"cran-comments.md"
)
writeLines("# test environment\n", "cran-comments.md")
local_mocked_bindings(git_default_branch = function() "master")

expect_snapshot({
local_edition(2L)
writeLines(upkeep_checklist())
},
transform = scrub_checklist_footer
)
})
})

test_that("get extra upkeep bullets works", {
env <- env(upkeep_bullets = function() c("extra", "upkeep bullets"))
expect_equal(upkeep_extra_bullets(env),
c("* [ ] extra", "* [ ] upkeep bullets", ""))
expect_equal(
upkeep_extra_bullets(env),
c("* [ ] extra", "* [ ] upkeep bullets", "")
)

env <- NULL
expect_equal(upkeep_extra_bullets(env), "")
Expand Down
Loading