New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Letterfoot #23
Letterfoot #23
Conversation
@@ -0,0 +1,13 @@ | |||
# remotes::install_github("coolbutuseless/minipdf") | |||
library(minipdf) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do without additional depends? I.e. place a pre-made pdf somewhere -- ie in vignettes/examples
and don't create it on the fly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and I see you added the file, so this is "mostly" just documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this just shows how the footer was created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do without it if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If R CMD check
does not complain about it (and I later realized it would not as this not package code per se) then it can stay, and is in fact a nice touch documenting how you made it.
I was just being careful and there is (in my book) no upside to expanding Depends: or Imports: for pure illustrations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks. Sure. I myself definitely subscribe to limiting dependencies as much as possible too.
Yes, please add that line as well. |
One more please: Add yourself and the changed files to |
Updated |
Just tested this out, works great! Thanks for the PR, @mbojan. |
Sorry, week got crazy busy. Merging now, and thanks to @aaronwolen for test driving it. |
This addresses #10 .
I have updated the docs and the vignette too.
BTW,
R CMD check
complains that it needsEncoding: UTF-8
in theDESCRIPTION
. May I fix this on in this PR before the (hopeful) merge?