search

Bioconductor / bioconductor_docker

Docker Containers for Bioconductor
https://bioconductor.org/help/docker/
Artistic License 2.0
73 stars 31 forks source link

sed IS_BIOC_BUILD_MACHINE to false #60

Closed almahmoud closed 2 years ago

hpages commented 2 years ago 
sed -i 's/IS_BIOC_BUILD_MACHINE=true/IS_BIOC_BUILD_MACHINE=false/g' Renviron.bioc

That looks fragile. What if at some point in the future someone introduces a space between = and true, or replaces true with True? Also I would not be surprised if some packages around don't bother to actually check the value of IS_BIOC_BUILD_MACHINE and just do something like if (!identical(Sys.getenv("IS_BIOC_BUILD_MACHINE"), "")) ... to detect that they are on a BioC builder.

So maybe a more robust approach would be to simply remove any line that starts with IS_BIOC_BUILD_MACHINE?

almahmoud commented 2 years ago

I think the sed would be only on this file, so any change would be on our side not from package developers? In any case, I agree removing altogether is better, so I'll change it to a delete

hpages commented 2 years ago

I think the sed would be only on this file, so any change would be on our side not from package developers?

Correct. But we sometimes need to edit this file and whoever is going to edit it in the future wouldn't know they break your code by just introducing a space somewhere or replacing true with True. We want to be safe even against our own goofs :stuck_out_tongue_winking_eye:

Thanks for the change!