search

Appsilon / rhino

Build high quality, enterprise-grade Shiny apps at speed
https://appsilon.github.io/rhino
293 stars 25 forks source link

Renv dependencies.R file linter #553

Open radbasa opened 10 months ago

radbasa commented 10 months ago

Because renv does not see packages declared in box::use(), we use a dependencies.R file for renv to parse.

Does it make sense to check if the packages are sorted alphabetically in dependencies.R?

Will an alphabetical order cause issues?

kamilzyla commented 10 months ago

Note: I think renv now parses box::use() statements. But we might still prefer a solution with dependencies explicitly listed in a single file in project root. That's how some other major languages approach it (e.g. Python, JavaScript on Node.js) - probably because it makes sense to have this important information in a single place, and because scanning the whole repository each time this information is needed can be prohibitively slow. Anyway, it would be good to revisit our approach with dependencies.R - perhaps a better solution is possible today?

If we were to stick to dependencies.R, I think a linter to check for alphabetical order could be nice, but I do see one potential problem: users might choose to use comments and group packages in some meaningful way; alphabetical sorting would break it. I'm not sure if anybody does that though.

radbasa commented 10 months ago

Comments won't be a problem with lintr. But, grouping packages in some meaningful way does make sense (if anyone does that) and will be seen as lint if we choose to enforce alphabetically sorted library calls.