-
Notifications
You must be signed in to change notification settings - Fork 94
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
[MJAVADOC-653] fix javadoc; fix code smells #48
Conversation
4adc9b2
to
c994936
Compare
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.
The Travis CI should be pulled out to a separate issue and PR.
src/main/java/org/apache/maven/plugins/javadoc/AbstractFixJavadocMojo.java
Show resolved
Hide resolved
src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
Outdated
Show resolved
Hide resolved
c994936
to
ab752e1
Compare
@elharo done. commits about travis-ci has been rebased out. |
src/main/java/org/apache/maven/plugins/javadoc/AbstractFixJavadocMojo.java
Outdated
Show resolved
Hide resolved
src/test/java/org/apache/maven/plugins/javadoc/FixJavadocMojoTest.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java
Outdated
Show resolved
Hide resolved
src/test/java/org/apache/maven/plugins/javadoc/AggregatorJavadocReportTest.java
Outdated
Show resolved
Hide resolved
@@ -154,7 +154,7 @@ public void testInvalidDestdir() | |||
//check if the javadoc jar file was generated | |||
File generatedFile = new File( getBasedir(), | |||
"target/test/unit/javadocjar-invalid-destdir/target/javadocjar-invalid-destdir-javadoc.jar" ); | |||
assertTrue( !FileUtils.fileExists( generatedFile.getAbsolutePath() ) ); | |||
assertFalse(FileUtils.fileExists(generatedFile.getAbsolutePath())); |
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.
Maven style uses spaces before ) and after ( unless there are no arguments
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.
@elharo done.
btw, is there a code formatter / style checker for this code for this format we used here?
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.
please check here: https://maven.apache.org/developers/conventions/code.html
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.
please check here: https://maven.apache.org/developers/conventions/code.html
@slachiewicz Thanks.
btw is there anybody for more reviewings to this pr?
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.
Can you put numbers on the performance improvement?
@elharo sorry but I didn't get what you mean by "put numbers on". |
Absent measurement, it can't be claimed that a PR improves performance; and measuring these things is hard. |
OK, if you see it this way I will not oppose.
Indeed. A good usecase for performance test can be very hard to built... |
@elharo |
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.
Yes, if you could rebase and squash your commits that would be wonderful. Thanks.
1da9f85
to
1696b91
Compare
1696b91
to
5bd700c
Compare
@elharo |
Running it through Jenkins now: https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-javadoc-plugin/job/MJAVADOC-653/ |
@elharo |
Please file a JIRA issue for each actual bug. |
OK. |
No description provided.