search

mcguinlu / robvis

A package to quickly visualise risk-of-bias assessment results
https://mcguinlu.github.io/robvis/
Other
58 stars 22 forks source link

Properly document functions in helpers.R #108

Closed mcguinlu closed 1 year ago

mcguinlu commented 3 years ago

Under time-pressure and so the docs suffered! 🤦‍♂️

All the functions in the helpers.R file currently do not have any associated documentation - this makes it hard for new contributors (let alone me!) to see what's going on, and so acts a barrier to contributing.

Very happy for anyone who is looking for a good first issue to have a look at this!

AJFOWLER commented 3 years ago

I'll do some work on this Luke as I'm sure some of the mess is my fault!

Do you have strong feelings about how it's done? I could either document multiple functions in one file, split them out into helper_*.R files individually, or just use comments!

mcguinlu commented 3 years ago

Hah, I actually think your bits were reasonably fine, but then I chucked mine in on top.

Absolutely fab if you had some time to run through it and add/improve the roxygen docs - it'll be a massive help as we work towards v1.0.0, and having someone else try and interpret the helper functions will definitely cause some bugs to shake out (particularly the validation ones).

Based on the current approach, I'm not sure I have a right to be fussy, but I would prefer not to use the shared documentation for multiple functions (as per your linked example). I find it works well with single line functions (as in the example) because it's easy to see what the shared arguments are, but once you get up to larger functions, it requires a lot of scrolling.

Re: individual helper_*.R files vs commented single file, no real preference, as I use the "Jump to function definition" rather than navigating via files.

One minor thing (which I'm sure you know, but just in case) is to make sure to tag them with "internal" so pkgdown doesn't try and build reference files for them:

#' @keywords internal

Thanks (as always) @AJFOWLER!