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

devtools::check doesn't catch invalid file URIs #2054

Closed
IndrajeetPatil opened this issue Jun 4, 2019 · 10 comments
Closed

devtools::check doesn't catch invalid file URIs #2054

IndrajeetPatil opened this issue Jun 4, 2019 · 10 comments

Comments

@IndrajeetPatil
Copy link
Contributor

IndrajeetPatil commented Jun 4, 2019

Although I had 0 NOTE, ERROR, or WARNING on my local machine, this is the NOTE I am getting with the development version of R (but not on R 3.6.0).

Found the following (possibly) invalid file URIs:
  URI: commits/master
    From: README.md
  URI: /commits/master
    From: README.md
  URI: CONDUCT.md
    From: README.md

This is my .Rbuildignore for the R package in question-
https://github.com/IndrajeetPatil/ggstatsplot/blob/master/.Rbuildignore

I am guessing this is a new check implemented on R CMD CHECK? Maybe worth including in devtools as well?

P.S. I am also attaching the full log in case you need to have a look it.
full_log.txt

@jimhester
Copy link
Member

devtools::check() is just a wrapper around R CMD check, so to get this check you need to check with R devel, and also use remote = TRUE.

@IndrajeetPatil
Copy link
Contributor Author

Got it. But how can I get rid of this NOTE? I have no clue what the problem is here.

@jennybc
Copy link
Member

jennybc commented Jun 4, 2019

Replace any internal links in your README with a full URL to GitHub (or perhaps a pkgdown site).

@IndrajeetPatil
Copy link
Contributor Author

That solved it! Thanks.

I still think devtools or rcmdcheck should catch this though.

@jennybc
Copy link
Member

jennybc commented Jun 4, 2019

I still think devtools or rcmdcheck should catch this though.

Like @jimhester said, check() is just a wrapper around R CMD check at the end of the day and this is a brand new check added by CRAN. This check doesn't exist in the current version of R. You won't see this unless you're checking with r-devel. Assuming you don't want to install r-devel locally, you can check on r-devel remotely with devtools::check_win_devel(), on r-hub, or on travis.

@IndrajeetPatil
Copy link
Contributor Author

Ah, got it. Sorry when he said it's a wrapper around R CMD CHECK, I somehow converted that to rcmdcheck and thought to myself why not implement this check in that package.

I understand now why this is not possible.

@IndrajeetPatil
Copy link
Contributor Author

I am guessing for the same reason devtools also doesn't catch NOTE related to invalid URLs:

URL: https://cran.r-project.org/web/packages/ggstatsplot/readme/README.html
    From: README.md
    Status: 200
    Message: OK
    CRAN URL not in canonical form
  URL: https://cran.r-project.org/web/packages/ggstatsplot/vignettes/
    From: README.md
    Status: 200
    Message: OK
    CRAN URL not in canonical form
  The canonical URL of the CRAN page for a package is 
    https://CRAN.R-project.org/package=pkgname

@jimhester
Copy link
Member

I you running check with remote = TRUE? those checks are only turned on when that is the case.

@IndrajeetPatil
Copy link
Contributor Author

Yes, this is the exact code I am running-

devtools::check(
  run_dont_test = TRUE,
  manual = TRUE,
  remote = TRUE,
  incoming = TRUE,
  force_suggests = TRUE
)

@lock
Copy link

lock bot commented Dec 7, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Dec 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants