-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor url assertions #1
Conversation
Also, I replaced pkgsearch::ps() with the more specialized pkgsearch::cran_package(). I noticed issues with the former, e.g. ps("crew", size = 1) returns info on |
Also: I was using shaky grep patterns before this PR, and I really appreciate being able to use |
I also added a check to flag manual review if a user contributes a URL which uses the CRAN mirror https://github.com/cran/ |
This is great - much more robust! Yes, the changes I put through were meant to add the functionality quickly - I'm sure a lot can be optimized down this road. |
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 subject to my comments. I don't need to review again before merging if just these.
Thanks for your review, @shikokuchuo. I believe I addressed all your comments. |
LGTM |
use calendar month for staging period
@shikokuchuo, I read what you did in the last couple commits, and I really like it. In the future though, it would really help to submit pull requests when we make changes to parts of
r-releases
, especially this package.This PR is a refactor and enhancement of URL assertions. In particular:
assert_cran_url()
fromassert_package_url()
to makereview_pull_request()
simpler.I would strongly prefer to avoid modifying
review_pull_request()
andreview_pull_requests()
as much as possible because these functions call the API and are much harder to test. This PR revertsreview_pull_request()
back to pretty much the way it was, exceptassert_package_contents()
is now replaced byassert_package()
.