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

[R-package] [docs] replace build_r_site.R with pkgdown::build_site() #3098

Merged
merged 4 commits into from
May 17, 2020

Conversation

jameslamb
Copy link
Collaborator

Per this conversation (#1143 (comment)), this PR removes build_r_site.R and replaces it with a call to pkgdown::build_site(install = FALSE).

Documentation tools like roxygen2::roxygenize(), pkgdown::build_site(), and devtools::document() used to try to rebuild R packages and this work well with ours. But now these tools support installing once and then running their functionality without re-building. So this flow works and only builds the package one time:

Rscript build_r.R

Rscript -e "roxygen2::royxgenize(load = 'installed')"

Rscript -e "pkdown::build_site()"

I tried building this on readthedocs and it looks ok!

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Only two minor points below for discussion.

docs/conf.py Show resolved Hide resolved
build_r_site.R Show resolved Hide resolved
@StrikerRUS
Copy link
Collaborator

I think doc PR label is for something that is changing any documentation for users. This PR leaves everything visible for users the same. So I believe maintaince label should be used for invisible at user side changes.

@jameslamb jameslamb added maintenance and removed doc labels May 17, 2020
@jameslamb jameslamb force-pushed the fix/remove-devtools branch from 154fd0b to c428cb0 Compare May 17, 2020 16:31
@jameslamb
Copy link
Collaborator Author

After pinning new_process in c428cb0, can confirm this is still working:

I'll merge this if CI build successfully

@jameslamb jameslamb merged commit bf99a07 into master May 17, 2020
@jameslamb
Copy link
Collaborator Author

@StrikerRUS now that this is merged, you can disable the fix/remove-devtools branch on readthedocs and I'll remove it from the repo here. Thanks for the help!

@jameslamb jameslamb deleted the fix/remove-devtools branch May 17, 2020 17:57
@StrikerRUS
Copy link
Collaborator

pkgdown::build_site() creates a docs directory automatically:

Ah, makes perfect sense! Completely missed it.

now that this is merged, you can disable the fix/remove-devtools branch on readthedocs and I'll remove it from the repo here.

OK. Done.

Thank you very much for this great simplification!

odimka pushed a commit to odimka/LightGBM that referenced this pull request May 17, 2020
…icrosoft#3098)

* [R-package] [docs] replace build_r_site.R with pkgdown::build_site()

* fix paths

* fix continuation

* new_process arg
ChipKerchner pushed a commit to ChipKerchner/LightGBM that referenced this pull request Jun 10, 2020
…icrosoft#3098)

* [R-package] [docs] replace build_r_site.R with pkgdown::build_site()

* fix paths

* fix continuation

* new_process arg
ChipKerchner pushed a commit to ChipKerchner/LightGBM that referenced this pull request Jun 11, 2020
…icrosoft#3098)

* [R-package] [docs] replace build_r_site.R with pkgdown::build_site()

* fix paths

* fix continuation

* new_process arg
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants