-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[docs] fix R documentation builds (fixes #3655) #3656
Conversation
docs/conf.py
Outdated
@@ -248,8 +248,9 @@ def generate_r_docs(app): | |||
source /home/docs/.conda/bin/activate r_env | |||
export TAR=/bin/tar | |||
cd {0} | |||
git submodule update --init --recursive |
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.
Thanks for the fix! I think it will be better to use config file for such things.
https://docs.readthedocs.io/en/stable/config-file/v2.html#submodules
Could you please try include the following lines
submodules:
include: all
recursive: true
into
https://github.com/microsoft/LightGBM/blob/master/.readthedocs.yml?
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.
P.S. I've enabled RTD builds for the docs/jlamb
branch and will keep it turned on as you'd asked.
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.
sure, I can do that. And thanks for keeping the branch up!
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.
docs/conf.py
Outdated
export R_LIBS="$CONDA_PREFIX/lib/R/library" | ||
Rscript build_r.R | ||
Rscript build_r.R || exit 1 |
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.
Please use -1
for the consistency. It may help to simply a search query in the repo if we decide to change something in the future.
Shouldn't Rscript -e "pkgdown::build_site()
cause builds to fail as well?
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 you're right
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.
Thanks for updating the config and forcing build to fail!
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. |
This PR attempts to fix the broken R documentation (see #3655). Building the docs requires installing the R package. Now that #3405 has been merged, LightGBM's submodules have to be initialized for the package to be built successfully, and I think they were not on readthedocs builds.
latest build: https://readthedocs.org/projects/lightgbm/builds/12567523/
and then later
Notes for Reviewers
@StrikerRUS could you please enable readthedocs builds for this branch?