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

Jump to bottom

[MJAVADOC-653] fix javadoc; fix code smells #48

Merged
merged 1 commit into from Aug 19, 2020
Merged

[MJAVADOC-653] fix javadoc; fix code smells #48

merged 1 commit into from Aug 19, 2020

Conversation

XenoAmess
Copy link
Contributor

No description provided.

@XenoAmess XenoAmess changed the title fix javadoc; fix code smells; performance improvement; add travis-ci script. [MJAVADOC-653] fix javadoc; fix code smells; performance improvement; add travis-ci script. May 29, 2020
elharo
elharo reviewed Jun 6, 2020
View reviewed changes
Copy link
Contributor

@elharo elharo left a 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.

@XenoAmess
Copy link
Contributor Author

The Travis CI should be pulled out to a separate issue and PR.

@elharo done. commits about travis-ci has been rebased out.

@XenoAmess XenoAmess changed the title [MJAVADOC-653] fix javadoc; fix code smells; performance improvement; add travis-ci script. [MJAVADOC-653] fix javadoc; fix code smells; performance improvement Jun 9, 2020
@XenoAmess XenoAmess requested a review from elharo June 20, 2020 03:29
@XenoAmess XenoAmess requested a review from elharo June 20, 2020 12:24
elharo
elharo requested changes Jun 20, 2020
View reviewed changes
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
src/test/java/org/apache/maven/plugins/javadoc/JavadocJarTest.java Outdated
@@ -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()));
Copy link
Contributor

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

Copy link
Contributor Author

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?

Copy link
Member

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

Copy link
Contributor Author

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?

@XenoAmess XenoAmess requested a review from elharo June 20, 2020 13:26
elharo
elharo reviewed Jul 20, 2020
View reviewed changes
Copy link
Contributor

@elharo elharo left a 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?

@XenoAmess
Copy link
Contributor Author

Can you put numbers on the performance improvement?

@elharo sorry but I didn't get what you mean by "put numbers on".
If you need performance test then I think I can try make some jmh tests.
But if you need real-time usecases performance test then I can't help with that.

@elharo elharo changed the title [MJAVADOC-653] fix javadoc; fix code smells; performance improvement [MJAVADOC-653] fix javadoc; fix code smells Jul 20, 2020
@elharo
Copy link
Contributor

elharo commented Jul 20, 2020

Absent measurement, it can't be claimed that a PR improves performance; and measuring these things is hard.

@XenoAmess
Copy link
Contributor Author

XenoAmess commented Jul 20, 2020 •
edited

@elharo

Absent measurement, it can't be claimed that a PR improves performance;

OK, if you see it this way I will not oppose.
But I thought changes I made in JavadocUtil.java is easy to be think a performance refine.

and measuring these things is hard.

Indeed. A good usecase for performance test can be very hard to built...

@XenoAmess
Copy link
Contributor Author

@elharo
Hi.
What should we do next for this pr?
Should I sqruash all commits now?

elharo
elharo reviewed Aug 17, 2020
View reviewed changes
Copy link
Contributor

@elharo elharo left a 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.

@XenoAmess
Copy link
Contributor Author

Yes, if you could rebase and squash your commits that would be wonderful. Thanks.

@elharo
rebase done.

@elharo
Copy link
Contributor

elharo commented Aug 17, 2020

Running it through Jenkins now: https://ci-builds.apache.org/job/Maven/job/maven-box/job/maven-javadoc-plugin/job/MJAVADOC-653/

@elharo elharo merged commit 71fb290 into apache:master Aug 19, 2020
@XenoAmess
Copy link
Contributor Author

XenoAmess commented Aug 19, 2020 •
edited

@elharo
Thanks.
As the codes are clean now, I can continue to the next step.
The next pr will be some fix about some bugs in javadoc:fix .
I will try to do it as fast as possible.
Will create pr when I finish.

@elharo
Copy link
Contributor

elharo commented Aug 19, 2020

Please file a JIRA issue for each actual bug.

@XenoAmess
Copy link
Contributor Author

Please file a JIRA issue for each actual bug.

OK.

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

@elharo elharo elharo approved these changes

@slachiewicz slachiewicz slachiewicz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@XenoAmess @elharo @slachiewicz