-
Notifications
You must be signed in to change notification settings - Fork 413
[client/flink] generate javadoc for public modules #1724
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
base: main
Are you sure you want to change the base?
Conversation
Thanks for the great work @MehulBatra ! Here are my thoughts about the javadoc design:
Here are suggestions about how to implement:
|
Thanks for the feedback @wuchong , I agree on all the points will implement on these lines! |
Github Action PR: apache/fluss-website#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.
Pull Request Overview
This PR implements javadoc generation for Apache Fluss's public API modules. The changes add a Maven-based javadoc build system that generates comprehensive API documentation for client and Flink integration modules while excluding internal implementation details.
- Added automated javadoc generation script with version-aware output structure
- Configured Docusaurus navigation to include Javadocs link
- Cleaned up minor formatting issues in root pom.xml
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
website/docusaurus.config.ts | Added Javadocs navigation link to website header |
website/build_javadocs.sh | New script for generating javadocs with Maven, branch-aware versioning, and redirect page creation |
pom.xml | Minor formatting cleanup removing extra blank lines |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
{to: '/community/welcome', label: 'Community', position: 'left'}, | ||
{to: '/roadmap', label: 'Roadmap', position: 'left'}, | ||
{to: '/downloads', label: 'Downloads', position: 'left'}, | ||
{to: '/javadoc', label: 'Javadocs', position: 'left', target: '_blank'}, |
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.
Extra space before 'target: '_blank'' - there should be only one space after the comma for consistent formatting.
{to: '/javadoc', label: 'Javadocs', position: 'left', target: '_blank'}, | |
{to: '/javadoc', label: 'Javadocs', position: 'left', target: '_blank'}, |
Copilot uses AI. Check for mistakes.
VERSION=$(./mvnw help:evaluate -Dexpression=project.version -q -DforceStdout 2>/dev/null || mvn help:evaluate -Dexpression=project.version -q -DforceStdout) | ||
if [[ -z "$VERSION" ]]; then | ||
echo "Error: Could not extract project version" |
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.
[nitpick] The command fallback logic could fail silently if both mvnw and mvn commands fail but return empty output. Consider adding explicit error handling to detect when both commands fail.
VERSION=$(./mvnw help:evaluate -Dexpression=project.version -q -DforceStdout 2>/dev/null || mvn help:evaluate -Dexpression=project.version -q -DforceStdout) | |
if [[ -z "$VERSION" ]]; then | |
echo "Error: Could not extract project version" | |
# Try to get version with ./mvnw first | |
VERSION="" | |
if [[ -x "./mvnw" ]]; then | |
VERSION=$(./mvnw help:evaluate -Dexpression=project.version -q -DforceStdout 2>/dev/null) | |
MVNW_STATUS=$? | |
else | |
MVNW_STATUS=127 | |
fi | |
if [[ $MVNW_STATUS -ne 0 || -z "$VERSION" ]]; then | |
VERSION=$(mvn help:evaluate -Dexpression=project.version -q -DforceStdout 2>/dev/null) | |
MVN_STATUS=$? | |
else | |
MVN_STATUS=0 | |
fi | |
if [[ ($MVNW_STATUS -ne 0 && $MVN_STATUS -ne 0) || -z "$VERSION" ]]; then | |
echo "Error: Could not extract project version using either ./mvnw or mvn." |
Copilot uses AI. Check for mistakes.
if (./mvnw -f "$TEMP_POM" clean compile javadoc:aggregate -q 2>/dev/null || mvn -f "$TEMP_POM" clean compile javadoc:aggregate -q); then | ||
echo "Javadoc generation completed successfully" | ||
else | ||
echo "Error: Javadoc generation failed" |
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.
[nitpick] Similar to the version extraction, this Maven command fallback could benefit from more explicit error handling. The current approach may not distinguish between command not found vs actual build failures.
if (./mvnw -f "$TEMP_POM" clean compile javadoc:aggregate -q 2>/dev/null || mvn -f "$TEMP_POM" clean compile javadoc:aggregate -q); then | |
echo "Javadoc generation completed successfully" | |
else | |
echo "Error: Javadoc generation failed" | |
# Explicit Maven command fallback with error handling | |
if [[ -x "./mvnw" ]]; then | |
echo "Using ./mvnw to build and generate Javadoc..." | |
./mvnw -f "$TEMP_POM" clean compile javadoc:aggregate -q | |
MVN_EXIT_CODE=$? | |
if [[ $MVN_EXIT_CODE -ne 0 ]]; then | |
echo "Error: Javadoc generation failed using ./mvnw (exit code $MVN_EXIT_CODE)" | |
exit 1 | |
fi | |
elif command -v mvn >/dev/null 2>&1; then | |
echo "Using mvn to build and generate Javadoc..." | |
mvn -f "$TEMP_POM" clean compile javadoc:aggregate -q | |
MVN_EXIT_CODE=$? | |
if [[ $MVN_EXIT_CODE -ne 0 ]]; then | |
echo "Error: Javadoc generation failed using mvn (exit code $MVN_EXIT_CODE)" | |
exit 1 | |
fi | |
else | |
echo "Error: Neither ./mvnw nor mvn found. Please install Maven or ensure ./mvnw is present." |
Copilot uses AI. Check for mistakes.
Purpose
Linked issue: close #1712
Implement javadoc generation
Brief change log
Tests
Manual testing of javadoc generation script with different scenarios
Verified generated HTML output structure and content
Validated redirect functionality and version-specific URL routing
Run:
API and Format
No API changes - This is purely a build/documentation improvement that doesn't affect runtime APIs.
Documentation
The generated javadoc itself serves as API reference documentation.