ISAAKiel / oxcAAR

R Package - Interaction with Oxcal
GNU General Public License v2.0
21 stars 4 forks source link

Quickfix download and cran checks #44

Closed MartinHinz closed 3 years ago

MartinHinz commented 3 years ago

We were asked to make the quicksetup function fail graceful if oxcal is not downloadable. I implemented the required features, as described in #43, in this branch.

MartinHinz commented 3 years ago

Since you, @nevrome, originally have written this part, could you please be so kind as to check my alterations?

codecov-commenter commented 3 years ago

Codecov Report

Merging #44 (c335c5e) into master (2578090) will decrease coverage by 0.36%. The diff coverage is 56.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   66.59%   66.22%   -0.37%     
==========================================
  Files          13       13              
  Lines         898      909      +11     
==========================================
+ Hits          598      602       +4     
- Misses        300      307       +7     
Impacted Files Coverage Δ
R/utility_functions.R 80.00% <56.25%> (-7.50%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2578090...c335c5e. Read the comment docs.

nevrome commented 3 years ago

Hm - shouldn't this error then also be handled in quickSetupOxcal? Without an internet connection this user facing function still fails rather unpleasantly:

Oxcal doesn't seem to be installed. Downloading it now:
trying URL 'https://c14.arch.ox.ac.uk/OxCalDistribution.zip'
Error Downloading OxCalDistribution.zip:
URL 'https://c14.arch.ox.ac.uk/OxCalDistribution.zip': status was 'Couldn't resolve host name'
No internet connection or data source broken?

Error in setOxcalExecutablePath(exe) : No file at given location 

If downloadOxcal fails, then the function should stop there without running setOxcalExecutablePath.

MartinHinz commented 3 years ago

Thank you for pointing this out! Actually, I philosophically would expect an error here, and not a rather silent warning thingy. I can not understand the CRAN Policy at this point. Never the less, f7be88e introduces additional catch for this situation? Is that ok for you?