Skip to content
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] [R-package] run 'R CMD check' as a foreground task #6508

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Jun 28, 2024

Contributes to #6498

4 years ago, this project was running a lot of its R-package testing on Travis CI. We experienced an issue where R CMD check would run for too long without emitting a new log line, and as a result Travis killed jobs.

A workaround was introduced in #3092 ... running R CMD check as a background task and polling for the status of it's process ID.

I strongly suspect this is no longer necessary:

  • we are not on Travis any more (and I suspect that GitHub Actions does not have a similar timeout based on log output)
  • the R package compiles faster in CI than it did 4 years ago

This proposes removing that workaround, to hopefully make it a bit easier to understand the CI setup. It also fixes some shellcheck errors, mentioned in #6501 (comment)

How I tested this

Checked a few of the logs to be sure R CMD check is really being run and is succeeding. Looks good to me!

https://github.com/microsoft/LightGBM/actions/runs/9788164564/job/27025779285?pr=6508#step:9:2015

If it wasn't safe to remove this, we'd see jobs outright fail. I'm pretty confident this is safe to do.

@jameslamb jameslamb changed the title WIP: [ci] [R-package] run 'R CMD check' as a foreground task [ci] [R-package] run 'R CMD check' as a foreground task Jul 4, 2024
@jameslamb jameslamb marked this pull request as ready for review July 4, 2024 06:07
@jameslamb jameslamb merged commit 9a10b19 into master Jul 4, 2024
41 checks passed
@jameslamb jameslamb deleted the ci/r-cmd-check branch July 4, 2024 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants