fix override when included in as_pkgdown
#2406
Conversation
R/package.R
Outdated
| @@ -9,6 +9,11 @@ | |||
| #' @export | |||
| as_pkgdown <- function(pkg = ".", override = list()) { | |||
| if (is_pkgdown(pkg)) { | |||
| if (is.list(pkg$meta)) { | |||
There was a problem hiding this comment.
Thanks for digging this up! Can you please use modify_list() instead since that already handles the NULL case, which should simplify this code a bit 😄
There was a problem hiding this comment.
I was just copying the version a couple of lines down; I just changed that one to modify_list too for consistency sake
There was a problem hiding this comment.
oh, also, I was checking if x rather than y is null; there are cases when pkg$meta is NULL. I just included a change to modify_list that checks both arguments so we can use that instead like you were suggesting.
tests/testthat/test-init.R
Outdated
| @@ -46,3 +46,13 @@ test_that("site meta doesn't break unexpectedly", { | |||
|
|
|||
| expect_snapshot(yaml) | |||
| }) | |||
|
|
|||
| test_that("override works correctly for as_pkgdown", { | |||
There was a problem hiding this comment.
I'd suggest putting this test in test-package.R since that's corresponds to the file where you changed the code.
| @@ -86,7 +86,7 @@ isFALSE <- function(x) { | |||
| } | |||
|
|
|||
| modify_list <- function(x, y) { | |||
| if (is.null(y)) { | |||
There was a problem hiding this comment.
If x is NULL I think you should return y?
David's fix was merged into pkgdown and it was released (thank you @hadley)! r-lib/pkgdown#2406 Remove CI workaround.
Fixes #2257. I wasn't exactly sure where to put the test, but figured it was better to include a brief test somewhere.
As with the other issue, I actually ran into this doing
build_site_github_pages(), but figured it was better to fix at the root