Adding error handling to git_remote http requests (Resolves #625) #628
Conversation
R/install-git.R
Outdated
| download(tmp, url) | ||
| read_dcf(tmp)$Package | ||
| }, error = function(e) { | ||
| NA_character_ |
There was a problem hiding this comment.
| NA_character_ | |
| return(NA_character_) |
Yes I know that will just return 😄 But let's make it clear~
There was a problem hiding this comment.
In general we don't put explicit returns except for early returns. See https://style.tidyverse.org/functions.html#return for details.
|
Should be good to go - I just added a note to @niheaven's description of the HTTP changes. I felt that an additional NEWS entry would be unclear since it only pertains to new features in the upcoming release. Let me know if you'd prefer that it's a separate entry and I can split it out. |
|
@dgkf Maybe a separate entry is preferred, since my PR has already been merged into 2.4.0, and this PR sould be there in next version. |
|
Thanks @niheaven - mistake on my part. Didn't realize that code was already in a release. I reverted my edits and broke out the NEWS entry into the dev section. |
|
As far as I can tell, any errors on windows builds are hold overs from issues on current master. Let me know if there's anything more that you'd like me to address. |
|
I hope this PR can be incorporated soon, because this also resolves the issue regarding installing packages from Git repositories on AzureDevOps, see issue #621. |
Addressing #625
NAif either the download or DESCRIPTION parsing fails.remote_shasince it's not a real git url, but otherwise tests the http recovery).Happy to iterate from here, but felt it was simple enough to just get the ball rolling on it.