-
Notifications
You must be signed in to change notification settings - Fork 334
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
check_missing_topics() returns an error when Sys.getenv("CI") == "true" #1395
Conversation
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.
Looks good. A few comments in line. Thanks for working on this!
@hadley can you have a look why the test coverage is failing? I can't find the unit test referenced in the error. |
Looks good, thanks! You can ignore the test-coverage failure; I'm not sure what's going wrong but it's definitely not your fault. I'll merge this after I finish the 1.6.1 release (which I had to push out quickly since I accidentally broke cross-site article linking 😞) |
FYI @wch |
Think I just got bitten by this when rebuilding a package website on GitHub. Isn't it often the case that you might not want e.g. internal, non-exported functions to be listed in the package website's index, even though they might be documented internally and appear in the package's R help pages and so on? This change seems to assume we always want every function in the package to appear on the website index. This doesn't seem right, though perhaps I'm just mistaken about what I think is standard practice. |
@kjhealy I think the motivating principle is to keep the number of difference indices to a minimum; this change ensures that |
Closes #1378