[ci] [R-package] catch builds that have not updated docs #3205
Conversation
R-package/R/lgb.importance.R
Outdated
| @@ -1,5 +1,5 @@ | |||
| #' @name lgb.importance | |||
| #' @title Compute feature importance in a model | |||
| #' @title Compute feature importance in a model (testing) | |||
There was a problem hiding this comment.
I added this as an example of the type of change that should trigger a failure. I'll remove it once we see if the new task works as expected.
There was a problem hiding this comment.
it looks like this worked as expected!
https://github.com/microsoft/LightGBM/pull/3205/checks?check_run_id=836077809
| @@ -4,7 +4,7 @@ | |||
| CRAN_MIRROR="https://cloud.r-project.org/" | |||
| R_LIB_PATH=~/Rlib | |||
| mkdir -p $R_LIB_PATH | |||
| echo "R_LIBS=$R_LIB_PATH" > ${HOME}/.Renviron | |||
| export R_LIBS=$R_LIB_PATH | |||
There was a problem hiding this comment.
I made a mistake while developing...code with Rscript --vanilla -e failed to load a library because --vanilla ignores the .Renviron
I think we should remove this and use normal environment variables instead, to make the code a little more explicit and easier to reason about.
.ci/test.sh
Outdated
| @@ -69,7 +69,7 @@ if [[ $TASK == "if-else" ]]; then | |||
| exit 0 | |||
| fi | |||
|
|
|||
| if [[ $TASK == "r-package" ]]; then | |||
| if [[ $TASK == "r-package" ]] || [[ $TASK == "check-r-docs" ]]; then | |||
There was a problem hiding this comment.
Just a suggestion to not overcomplicate this if statement in the future. Name task with something like r-package-check-docs and then just check that it starts with r-package.
There was a problem hiding this comment.
oh good idea! Just pushed 0b17315 to implement this
Co-authored-by: Nikita Titov <[email protected]>
|
@guolinke Please make new job required. Thank you! |
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
From @StrikerRUS 's suggestion in #3188 (comment), this pull request proposes adding a new R CI jobs called
check-r-docs. This job regenerates the documentation for the R package and checks if there are any changes that have not been committed to the repo.In R, the process for builds the docs in the
R-package/man/folder is like this:R-package/R/roxygen2::roxygenize(load = 'installed')Because this process relies on installing the library, I think it makes sense to add into
./.ci/test_r_package.sh. It requires installing one additional somewhat-expensive dependency (roxygen2) and the documentation is not dependent on operating system so I think it makes sense to do this in a single new TASK instead of running it in all of the R CI jobs.