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

Avoid creation of faulty site #2439

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Avoid creation of faulty site #2439

merged 5 commits into from
Apr 15, 2024

Conversation

olivroy
Copy link
Collaborator

@olivroy olivroy commented Apr 12, 2024

Fixes #2329.

Supersedes #2411

Applies the suggestions to call init_site() earlier.

Will be interesting to see the codecov patch, maybe some messages are no longer reached?

Errors are caused by the fact that init_site() is called earlier, which triggers a message.

R/utils-fs.R Outdated Show resolved Hide resolved
@olivroy
Copy link
Collaborator Author

olivroy commented Apr 12, 2024

@hadley solved. I tripped on the fact that having a default argument the same as the name

i.e.

f <- function(x = x) {
}

creates problem. What is the reason behind that?

@hadley
Copy link
Member

hadley commented Apr 12, 2024

I forget the details, but because default arguments are evaluated when they're used. For example, this is valid R code:

f <- function(x = y) {
  y <- 10
  x
}

R/utils-fs.R Outdated Show resolved Hide resolved
"{.file {pkg$dst_path}} is non-empty and not built by pkgdown",
"!" = "Make sure it contains no important information \\
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I modified this error message, but it is almost impossible to get there now unless you really want to!

@hadley hadley merged commit 9f28b9f into r-lib:main Apr 15, 2024
12 of 13 checks passed
@hadley
Copy link
Member

hadley commented Apr 15, 2024

Thanks!

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.

FR: use more clickable hyperlinks in error messages
2 participants