Skip to content

Conversation

@ueshin
Copy link
Member

@ueshin ueshin commented Nov 2, 2014

No description provided.

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #22727 has started for PR 3058 at commit b548017.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 2, 2014

Test build #22727 has finished for PR 3058 at commit b548017.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22727/
Test PASSed.

@srowen
Copy link
Member

srowen commented Nov 2, 2014

I think we will ultimately have to turn this on, since javadoc 8 generates so much noise, but:

  • Are there javadoc errors that are fixable? I have tried to clean up a lot of them
  • Can we just disable warnings to cut the noise? or disable just some of the warnings instead of using :none?
  • Can we make errors not fail the build?

@ueshin
Copy link
Member Author

ueshin commented Nov 4, 2014

@srowen, Thank you for your comment.

The answers are:

  • Yes, there are 3 errors and 63 warnings and the errors, which are usage of HTML entities issues, are fixable and the warnings, which are mainly missing of Javadoc annotations like @param or @return, are fixable if developers write Javadocs correctly.
  • The checks are grouped and we can choose which groups should be checked, e.g. if we want to omit the missing issue mentioned above, we should use -Xdoclint:-missing insteand of -Xdoclint:none. The details are here. We have to make policies of Javadoc to choose the groups.
  • Yes, add configuration <failOnError>false</failOnError> to maven-javadoc-plugin instead of <additionalparam>-Xdoclint:none</additionalparam>, or combination of them if we choose check groups.

@srowen
Copy link
Member

srowen commented Nov 4, 2014

So instead, how about fixing the errors? then the build works, no? the warnings are annoying though, but can be disabled with just -missing then? Fixing errors and quieting this kind of warning seem better than completely turning off doclint.

@ueshin
Copy link
Member Author

ueshin commented Nov 4, 2014

Ah, I agree with you.
I could fix the errors and the build works now. And the warnings currently annoying go away with -missing.

@SparkQA
Copy link

SparkQA commented Nov 4, 2014

Test build #22877 has started for PR 3058 at commit 923e2f0.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 4, 2014

Test build #22877 has finished for PR 3058 at commit 923e2f0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22877/
Test PASSed.

@srowen
Copy link
Member

srowen commented Nov 4, 2014

@ueshin LGTM

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23873 has started for PR 3058 at commit 6762ec2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23873 has finished for PR 3058 at commit 6762ec2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23873/
Test PASSed.

Copy link
Member

Choose a reason for hiding this comment

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

@ueshin One last tiny thing. I think this plugin should be declared in <pluginManagement>, and should add <version>2.10.1</version>, but would not have <configuration> of course. Then this is left exactly as-is.

All -- I think this may help the problem in SPARK-4543? It is good to commit this fix anyway but may also simply silence Java 8's warnings on missing javadoc as desired.

Copy link
Member Author

Choose a reason for hiding this comment

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

@srowen Thank you for your suggestion and I agree with it. I'll add it to the <pluginManagement>.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23889 has started for PR 3058 at commit e096bb1.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23889 has finished for PR 3058 at commit e096bb1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23889/
Test PASSed.

@pwendell
Copy link
Contributor

This LGTM - thanks for the fix.

asfgit pushed a commit that referenced this pull request Nov 28, 2014
…rror.

Author: Takuya UESHIN <ueshin@happy-camper.st>

Closes #3058 from ueshin/issues/SPARK-4193 and squashes the following commits:

e096bb1 [Takuya UESHIN] Add a plugin declaration to pluginManagement.
6762ec2 [Takuya UESHIN] Fix usage of -Xdoclint javadoc option.
fdb280a [Takuya UESHIN] Fix Javadoc errors.
4745f3c [Takuya UESHIN] Merge branch 'master' into issues/SPARK-4193
923e2f0 [Takuya UESHIN] Use doclint option `-missing` instead of `none`.
30d6718 [Takuya UESHIN] Fix Javadoc errors.
b548017 [Takuya UESHIN] Disable doclint in Java 8 to prevent from build error.

(cherry picked from commit e464f0a)
Signed-off-by: Patrick Wendell <pwendell@gmail.com>
@asfgit asfgit closed this in e464f0a Nov 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants