mrkaye97 / slackr

An R package for sending messages from R to Slack
https://matthewrkaye.com/slackr/
Other
307 stars 84 forks source link

Add LaTex requirements in README #111

Closed xinye1 closed 3 years ago

xinye1 commented 3 years ago

110

New section LaTeX for version 2.0.0+ Under section Installation

Edit: also the Rmd is knitted to generate the md.

xinye1 commented 3 years ago

Nice work! Thanks for putting in this PR. If you don't mind spending the time, I think that adding a test to check if there's a LaTeX install (in the tests dir) makes sense. I can look into that if you don't want to. I'd also just add a tidbit about MacOS / MacTex to the LaTeX section too, just to be explicit about this potentially being an issue on MacOS too. Thanks again for these changes!

A test is a good idea, I'll have a look

Edit: on a second thought, as this is a texPreview issue, wouldn't it make more sense to fix it there? @yonicd I'd appreciate your thoughts. Thanks

yonicd commented 3 years ago

Requirements of texPreview are listed in the texPreview readme, you can point users to that.

If users need more info there is OS specific setups in the gha workflow ymls eg macos

mrkaye97 commented 3 years ago

Requirements of texPreview are listed in the texPreview readme, you can point users to that.

If users need more info there is OS specific setups in the gha workflow ymls eg macos

  • System Requirements:

    • Must have pdflatex in PATH, Windows users can install by running installr::install.MikTeX

    • TeX libraries that are used: standalone, xcolor, booktabs, multirow, array, helvet, amsmath, rotating, listings, graphicx, setspace, caption

    • To Check if the system is in compliance with these libraries run texPreview::check_requirements().

    • Ghostscript: If you are installing TeX with tinytex make sure to install ghostscript onto your system.

Ah, thanks for this. Didn't notice the texPreview readme. Seems like we can just add a test pretty easily to check texPreview::check_requirements()?

xinye1 commented 3 years ago

Thanks @yonicd, the gha lines are exactly what people need. I will add the ref to README

@mrkaye97 may I suggest a different approach? As TeX is only needed for tex_slackr (e.g. someone like me is unlikely to ever use it), it may be better to be lazy-loaded, e.g. only prompt the user to install / load texPreview when tex_slackr is called.

mrkaye97 commented 3 years ago

Thanks @yonicd, the gha lines are exactly what people need. I will add the ref to README

@mrkaye97 may I suggest a different approach? As TeX is only needed for tex_slackr (e.g. someone like me is unlikely to ever use it), it may be better to be lazy-loaded, e.g. only prompt the user to install / load texPreview when tex_slackr is called.

Sounds like a good approach to me! I don't have strong feelings either way, and I think lazy loading makes sense.

yonicd commented 3 years ago

lazy loading sounds like a good option