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.

Sign up for GitHub

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

Jump to bottom

YETUS-972. Make javadoc phase and goal configurable in maven.sh #112

Merged
merged 4 commits into from
Jun 25, 2020
Merged

YETUS-972. Make javadoc phase and goal configurable in maven.sh #112

merged 4 commits into from
Jun 25, 2020

Conversation

aajisaka
Copy link
Member

@aajisaka aajisaka mentioned this pull request Jun 24, 2020
Merged
@busbey
Copy link
Contributor

busbey commented Jun 24, 2020

Do you want to be able to use multiple goals? Like the example from HDFS-15424 was process-sources followed by a no fork javadoc?

To do things like that the cli arg needs to take a list of goals.

@aajisaka
Copy link
Member Author

Do you want to be able to use multiple goals?

Yes. Updated the patch to allow multiple goals from CLI. Thank you @busbey.

busbey
busbey requested changes Jun 24, 2020
View reviewed changes
precommit/src/main/shell/test-patch.d/maven.sh Outdated
@@ -26,7 +26,7 @@ MAVEN_CUSTOM_REPOS=false
MAVEN_CUSTOM_REPOS_DIR="@@@WORKSPACE@@@/yetus-m2"
MAVEN_DEPENDENCY_ORDER=true
MAVEN_FOUND_ROOT_POM=false
MAVEN_JAVADOC="javadoc:javadoc"
MAVEN_JAVADOC_GOALS="javadoc:javadoc"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be an array

precommit/src/main/shell/test-patch.d/maven.sh Outdated
--mvn-javadoc-goals=*)
delete_parameter "${i}"
MAVEN_JAVADOC_GOALS=${i#*=}
MAVEN_JAVADOC_GOALS=${MAVEN_JAVADOC_GOALS//,/ }
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use yetus_comma_to_array

precommit/src/main/shell/test-patch.d/maven.sh Outdated
@@ -335,7 +343,7 @@ function maven_modules_worker
modules_workers "${repostatus}" distclean clean -DskipTests=true
;;
javadoc)
modules_workers "${repostatus}" javadoc clean javadoc:javadoc -DskipTests=true
modules_workers "${repostatus}" javadoc clean "${MAVEN_JAVADOC_GOALS}" -DskipTests=true
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be "${MAVEN_JAVADOC_GOALS[@]}"

@aajisaka
Copy link
Member Author

Thank you for your quick review! Updated.

busbey
busbey approved these changes Jun 24, 2020
View reviewed changes
Copy link
Contributor

@busbey busbey left a comment

Choose a reason for hiding this comment

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

Please wait for qabot. LGTM.

@apache-yetus
Copy link

(!) A patch to the testing environment has been detected.
Re-executing against the patched versions to perform further tests.
The console is at https://builds.apache.org/job/yetus-github-multibranch/job/PR-112/1/console in case of problems.

@apache-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 24s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
_ Patch Compile Tests _
+1 💚 shellcheck 0m 3s There were no new shellcheck issues.
+1 💚 shelldocs 0m 0s There were no new shelldocs issues.
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
_ Other Tests _
+1 💚 asflicense 0m 10s The patch does not generate ASF License warnings.
1m 53s
Subsystem Report/Notes
Docker ClientAPI=1.40 ServerAPI=1.40 base: https://builds.apache.org/job/yetus-github-multibranch/job/PR-112/1/artifact/out/Dockerfile
GITHUB PR #112
Optional Tests dupname asflicense shellcheck shelldocs
uname Linux 97edb8290b13 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality precommit/src/main/shell/personality/yetus.sh
git revision master / 742d816
Max. process+thread count 41 (vs. ulimit of 2000)
modules C: precommit U: precommit
Console output https://builds.apache.org/job/yetus-github-multibranch/job/PR-112/1/console
versions git=2.7.4 maven=3.3.9 shellcheck=0.7.0
Powered by Apache Yetus 0.13.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

@aajisaka aajisaka merged commit f70b1e9 into apache:master Jun 25, 2020
@aajisaka aajisaka deleted the YETUS-972 branch June 25, 2020 01:45
@aajisaka
Copy link
Member Author

Merged. Thank you @busbey !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@busbey busbey busbey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aajisaka @busbey @apache-yetus