-
Notifications
You must be signed in to change notification settings - Fork 336
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
Display inst/AUTHORS
info in authors.html
#2506
Conversation
inst/AUTHORS
info in authors.htmlinst/AUTHORS
info in authors.html
R/build-home-authors.R
Outdated
@@ -2,6 +2,13 @@ data_authors <- function(pkg = ".", roles = default_roles()) { | |||
pkg <- as_pkgdown(pkg) | |||
author_info <- pkg$meta$authors %||% list() | |||
|
|||
inst_path <- path(pkg$src_path, "inst", "AUTHORS") | |||
if (length(inst_path) == 1 && file_exists(inst_path)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would the length not be 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmmm, good question.
@@ -29,6 +29,15 @@ test_that("source link is added to citation page", { | |||
expect_true(any(grepl("<code>inst/CITATION</code></a></small>", lines))) | |||
}) | |||
|
|||
test_that("citation page includes inst/AUTHORS", { | |||
pkg <- local_pkgdown_site(test_path("assets/inst-authors")) | |||
suppressMessages(init_site(pkg)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2330 will be nice 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw I'm not convinced it will be, because I think there are two meanings of quiet
currently. One is don't display anything (for functions where quiet = FALSE
as the default) and the other is "display more diagnostics" (for functions where quiet = TRUE
is the default)
expect_equal(data_authors_page(pkg)$before, "<p>Dream team:</p>") | ||
expect_equal(data_authors_page(pkg)$after, "<p>You are welcome!</p>") | ||
test_that("authors data includes inst/AUTHORS", { | ||
pkg <- as_pkgdown(test_path("assets/inst-authors")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would the test be easier to read if the test package were created here, with some test helper, and if the inst/authors file were then added inline in this test?
I thought there was an issue about test assets but I can't find it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe as part of #2509
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that would probably be the case for many of the tests, but requires quite a bit of test infrastructure investment. We'll see how I go.
@@ -2,6 +2,13 @@ data_authors <- function(pkg = ".", roles = default_roles()) { | |||
pkg <- as_pkgdown(pkg) | |||
author_info <- pkg$meta$authors %||% list() | |||
|
|||
inst_path <- path(pkg$src_path, "inst", "AUTHORS") | |||
if (length(inst_path) == 1 && file_exists(inst_path)) { | |||
inst <- read_lines(inst_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it need to be Markdownified? probably not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can assume this file is markdown, so the best we can do is put in a <pre>
tag.
Fixes #2332
To do: