-
Notifications
You must be signed in to change notification settings - Fork 952
Add script to generate javadoc with JDK17 #19170
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
Conversation
Signed-off-by: Yanxuan Liu <[email protected]>
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.
this should be applied to the mvn phase directly instead of providing another specific script
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
java/pom.xml
Outdated
<artifactId>maven-javadoc-plugin</artifactId> | ||
<version>3.6.3</version> | ||
<configuration> | ||
<classifier>javadoc-jdk17</classifier> |
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.
I think we were trying to override the default javadoc artifact, so rapids doc can simply use the default artifact for their pages instead of having another *-javadoc-jdk17 one
java/pom.xml
Outdated
@@ -358,6 +358,35 @@ | |||
</plugins> | |||
</build> | |||
</profile> | |||
<profile> | |||
<id>javadoc</id> |
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.
nit: rename it to javadoc-jdk17
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.
Pull Request Overview
This PR introduces a new Maven profile to generate javadoc using JDK17 and updates the Docker build script to optionally build the javadoc.
- Added a new in pom.xml for generating javadoc with JDK17.
- Updated java/ci/build-in-docker.sh to install Java 17 and trigger the javadoc build when enabled.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
java/pom.xml | New Maven profile to generate javadoc with JDK17 and custom configuration. |
java/ci/build-in-docker.sh | Added an option to install Java 17 and generate javadoc using the new profile. |
java/ci/build-in-docker.sh
Outdated
if [ $BUILD_JAVADOC_JDK17 == true ]; then | ||
yum install -y java-17-openjdk-devel | ||
export JDK17_HOME=/usr/lib/jvm/java-17-openjdk | ||
CUDF_INSTALL_DIR="$INSTALL_PREFIX" mvn -B javadoc:jar -P javadoc |
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.
Q: can we add the profile config directly to BUILD_ARG
when $BUILD_JAVADOC_JDK17 == true
? we do not require 2 javadoc generation steps
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
Signed-off-by: Yanxuan Liu <[email protected]>
/ok to test f0dd143 |
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.
LGTM
/merge |
Description
Checklist
fix #19062
Need to generate javadoc with JDK17 for search bar.
Created a script to generate javadoc to avoid modifying original pom.xml