-
Notifications
You must be signed in to change notification settings - Fork 36
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
[ci] Move CI to GitHub Actions (fixes #286) #287
Conversation
Codecov Report
@@ Coverage Diff @@
## main #287 +/- ##
=======================================
Coverage 92.60% 92.60%
=======================================
Files 12 12
Lines 946 946
=======================================
Hits 876 876
Misses 70 70 Continue to review full report at Codecov.
|
😱 😱 😱 |
ok I think this is ready for review! |
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.
One last comment but overall looks good to me. Thanks for going through this!
devscripts \ | ||
qpdf \ | ||
|| exit -1 | ||
fi |
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.
Did you check if we need these dependencies? I would've expected that between r-lib/actions/setup-r
, r-lib/actions/setup-pandoc
, and r-lib/actions/setup-tinytex
we'd be covered. If it's the case that we don't need this bash script, you could move the remotes
stuff into ci.yml
under run.
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.
I looked around tonight and found that r-lib/actions/setup-r
now seems to install qpdf
and checkbashisms
for you (it used to not), so I'll try removing those.
But some of these are needed by other packages in {pkgnet}
's dependencies. Only necessary on Linux because CRAN doesn't prepare precompiled binaries on Linux, so all those things need to be built from source.
libcurl4-openssl-dev \
libfribidi-dev \
libharfbuzz-dev \
curl \
Seconded. Thanks for this effort to fix the CI!! |
This PR moves
{pkgnet}
's CI from Travis to GitHub Actions, in response to the all the recent changes in Travis (see #286).Today on
main
, we only test with the latest version of R. I originally tried to add jobs for both R 4.0 and R 3.6 in this PR, but 3.6 was giving me some problems so I decided to cut that out of the scope for this PR.This PR also fixes this issue in the unit tests.
That fix should really be its own PR but I already did it here because while debugging, I think it might be related to soome issues I was facing.