search

eddelbuettel / rcppsimdjson

Rcpp Bindings for the 'simdjson' Header Library
115 stars 13 forks source link

(issue #64) add `always_list` parameter (fparse/fload) #65

Closed knapply closed 3 years ago

knapply commented 3 years ago

Resolves #64.

codecov[bot] commented 3 years ago

Codecov Report

Merging #65 (e7d5cd4) into master (32320b9) will increase coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   96.18%   96.21%   +0.02%     
==========================================
  Files          18       18              
  Lines        1363     1373      +10     
==========================================
+ Hits         1311     1321      +10     
  Misses         52       52
Impacted Files Coverage Δ
R/fload.R 100.00% <100.00%> (ø)
R/fparse.R 100.00% <100.00%> (ø)

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 32320b9...e7d5cd4. Read the comment docs.

knapply commented 3 years ago

Looks good as usual! I can deal with the codecov nag separately.

I think I would have put the new named argument at the end by some time honoured convention though ...

The codecov should be fixed momentarily. The rationale behind the argument order is that the arguments that are applicable to both fparse()/fload() go before the ones that are only applicable to fload(), but I can change that if desired.

eddelbuettel commented 3 years ago

Makes sense, I didn't pick up on that.

tdeenes commented 3 years ago

Thank you very much for the open-mindedness and the quick implementation!