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

Create a dockerized gradle task to update the generated R client documentation. #4713

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

jcferretti
Copy link
Member

@jcferretti jcferretti commented Oct 24, 2023

Towards #4689

This PR adds two new gradle tasks to R/build.gradle:

  • rClientDoc creates a docker image with R doc support and runs it in a container generating a tar bundle in build/man/man.tar.gz with the updated documentation.
  • updateRClientDoc overwrites the contents of R/rdeephaven/man with the newly generated doc by the task above.

You can run either (they have proper dependencies) like so, from the root of a deephave-core clone:
./gradlew :R:rClientDoc
./gradlew :R:updateRClientDoc

Notes

  • Something that is less than ideal in terms of dependencies is that R/roxygen2 (the package/tool that generates doc) requires the package that we intend to document (eg, rdeephaven) to be already installed. This implies we are forced to have the image that supports generating documentation depend on the image that builds the rdeephaven package; this is unfortunate because the image that supports generating documentation has to install a lot of base OS packages and then build the roxygen2 R package; all this does not depend on any way on our own rdeephaven sources, however, since the image depends on a base image that builds from those sources, any changes in rdeephaven sources will trigger dependencies in a way that will regenerate the documentation base image which will require reinstalling the OS base packages and roxygen2 in R, etc. We could make this less expensive by making all these additional OS packages and R roxygen package installation be part already of the cpp-clients-multi-base image from the deephaven-base-images repository... but then we will be taxing every deephaven developer who does not need to work on R related changes with a constant tax. So I think the tradeoff right now is to make this slightly less convenient to R developers only on the specific instance when doc needs to be regenerated in favor of avoiding that tax for everybody else on any and every change.

Copy link
Contributor

@alexpeters1208 alexpeters1208 left a comment

Choose a reason for hiding this comment

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

LGTM

@jcferretti jcferretti merged commit 054cc45 into deephaven:main Oct 25, 2023
14 checks passed
@jcferretti jcferretti deleted the cfs-gradle-update-r-doc branch October 25, 2023 20:16
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants