eddelbuettel / rcppsimdjson

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

This makes the scripts executable on a fresh R installation. #42

Closed lemire closed 4 years ago

lemire commented 4 years ago

This may not be a good idea, but for someone like myself who only use R from time-to-time, and often on a fresh R setup, having self-contained scripts that install their own dependencies is quality-of-life issue.

eddelbuettel commented 4 years ago

About to go for a (belated) run and will clean the commit up a little later. These days the recommended idiom is

if (!requireNamespace("somepackage", quietly=TRUE) doSomethingHere()

It is a matter of taste if make scripts install their requirements. I used to do it a little more ~20 years ago, I now find it (mostly) redundant as we a) do work with packages anyway and b) packages take care of this.

It is definitely useful for optional packages having a Suggests: in DESCRIPTION.

Lastly, compulsory advertisement: You want probably want littler to give you r (at least on real computers, those infected by some Cupertino-based disease need to alias to lr or some such as the brain surgeons over there default to r being equal to R. Grrrrrrr.)

lemire commented 4 years ago

This PR can be thrown away without hurting my feelings at all. ;-)

eddelbuettel commented 4 years ago

I added a small amount of spit and polish. Let me know what you think.

lemire commented 4 years ago

I think that this is much better!!! I recommend merging this.