b-rodrigues / rix

Reproducible development environments for R with Nix
https://b-rodrigues.github.io/rix/
GNU General Public License v3.0
102 stars 11 forks source link

un-clean starting script #138

Closed mmahmoudian closed 4 months ago

mmahmoudian commented 4 months ago

This is an interesting project, The name is just a bit of an issue when trying to find it on Github. Considering the existing packages on CRAN, NixR would be my suggestion (a Ctrl+f of "nix" get the user there which is good for visibility, plus it more inline with the CRAN style of naming ;) )

The issue

I thought of giving {rix} and Nix a try and as per suggestion of @b-rodrigues on Fediverse. Reading the README lead me to:

https://github.com/b-rodrigues/rix/blob/de009f777266a2836da26a5f3e118c7dcf911087/README.md?plain=1#L161-L168

And I thought since I'm on NixOS I give this approach a try. But before blindly running it and also for educational purposes I started reading the /inst/extdata/default.nix and then this caught my attention:

https://github.com/b-rodrigues/rix/blob/de009f777266a2836da26a5f3e118c7dcf911087/inst/extdata/default.nix#L35-L44

This package is not a dependency of {rix} not it is necessary for the code. I also realized the script also demands installation of rPackages.dplyr, and quarto, both of which are not necessary for starting an R project:

https://github.com/b-rodrigues/rix/blob/de009f777266a2836da26a5f3e118c7dcf911087/inst/extdata/default.nix#L22

https://github.com/b-rodrigues/rix/blob/de009f777266a2836da26a5f3e118c7dcf911087/inst/extdata/default.nix#L46

My suggestion for the solution

Move the testing bits to another file and keep this one pretty much lean and clean.

A proposal

If it is acceptable, I can make a PR and kickstart my contribution to this handy project.

b-rodrigues commented 4 months ago

@mmahmoudian thanks, you're right this should not be there! You can run this chunk to generate the right one

https://github.com/b-rodrigues/rix/blob/de009f777266a2836da26a5f3e118c7dcf911087/dev/0-dev_history.Rmd#L261

maybe put the latest commit as well, generate it an open a PR.

I think I know what happened: the chunk just below is the one that generated the wrong default.nix, so we should change overwrite to FALSE. As for the name, you're not wrong, we'll have to think about it.

Thanks!

b-rodrigues commented 4 months ago

Hi @mmahmoudian thanks again for opening the issue, I went ahead and fixed it with 4d96207e7f69cbadca5349e9caa5bcc4059c1dc3 :)