You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jodersky <gi...@git.apache.org> on 2016/04/26 20:57:17 UTC
[GitHub] spark pull request: Upgrade genjavadoc and use upstream version
GitHub user jodersky opened a pull request:
https://github.com/apache/spark/pull/12707
Upgrade genjavadoc and use upstream version
## What changes were proposed in this pull request?
In the past, genjavadoc had issues with package private members which led the spark project to use a forked version. This issue has been fixed upstream (typesafehub/genjavadoc#70) and a release is available for scala versions 2.10, 2.11 **and 2.12**, hence a forked version for spark is no longer necessary.
This pull request updates the build configuration to use the newest upstream genjavadoc.
## How was this patch tested?
The build was run `sbt unidoc`. During the process javadoc emits some errors on the generates java stubs, however these errors were also present before the upgrade. Furthermore, the produces html is fine.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jodersky/spark SPARK-14511-genjavadoc
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12707.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12707
----
commit f7d649c47d0d6f082b5a6237e4b60e8bd8cf3716
Author: Jakob Odersky <ja...@odersky.com>
Date: 2016-04-25T22:58:40Z
Upgrade genjavadoc and use upstream version
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12707
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215259124
@JoshRosen Are you using Java 8? I tested this on Java 7 with the latest master. It worked fine.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215809500
I created https://issues.apache.org/jira/browse/SPARK-15006 and https://github.com/typesafehub/genjavadoc/issues/83 to track the issue with package private objects.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215259445
Yeah, I think that was it. Forgot that I had pinned Java 7 for docs on Jenkins.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215257559
Actually, it looks like this might be broken even prior to this patch :(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-214851113
**[Test build #57016 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57016/consoleFull)** for PR 12707 at commit [`f7d649c`](https://github.com/apache/spark/commit/f7d649c47d0d6f082b5a6237e4b60e8bd8cf3716).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-214884017
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57016/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-214883726
**[Test build #57016 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57016/consoleFull)** for PR 12707 at commit [`f7d649c`](https://github.com/apache/spark/commit/f7d649c47d0d6f082b5a6237e4b60e8bd8cf3716).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215357914
Yeah, the problem is that Java 8 has a much stricter javadoc tool. `@group` isn't a valid javadoc tag, so when `ALSModel.scala` gets translated to Java to javadoc it, it fails. I am pretty sure this was one of several reasons I couldn't proceed along these lines -- that and the fact that the translation in genjavadoc would generate private top level classes, which isn't allowed in Java.
@jodersky you seem pretty plugged in to genjavadoc or am I imagining that? I think I had a PR open or at least a suggestion for fixing one of those.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215665077
Merged to master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by jodersky <gi...@git.apache.org>.
Github user jodersky commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215258121
Hmm, the current genjavadoc also produces errors so I didn't really look into it. How does the final output compare? Could this be due to java 8's stricter behaviour as @srowen mentioned?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215255118
I saw some errors running this locally though `jekyll`:
```
[error] /Users/joshrosen/Documents/spark/mllib/target/java/org/apache/spark/ml/recommendation/ALSModel.java:79: error: unknown tag: group
[error] /** @group setParam */
[error]
```
Any idea what's going on here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-214884014
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215260627
Though the doc compiles, it doesn't seem that the package private objects are hided. @jodersky Could you double check?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215006856
Seems reasonable to me if the upstream version does all that's needed.
I wonder if this new version happens to fix the problem in ... https://issues.apache.org/jira/browse/SPARK-3359
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by jodersky <gi...@git.apache.org>.
Github user jodersky commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215530295
@srowen, I did quite some digging through the genjavadoc codebase when re-implementing @mengxr's initial fix. I can try to fix the group warnings and object privacy issues, however I can't give you a fixed deadline since I won't be available until the 10th.
Since the Spark 2.0 freeze is coming up, do you still want to merge this (since it supports Scala 2.12)? I'll fix it regardless for a future version.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by jodersky <gi...@git.apache.org>.
Github user jodersky commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215010184
The work I submitted upstream doesn't specifically target that issue,
however it is actually plausible that it fixes the private interface issue
as a side effect. I haven't checked it though.
On Wed, Apr 27, 2016 at 1:14 AM, Sean Owen <no...@github.com> wrote:
> Seems reasonable to me if the upstream version does all that's needed.
> I wonder if this new version happens to fix the problem in ...
> https://issues.apache.org/jira/browse/SPARK-3359
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/12707#issuecomment-215006856>
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: Upgrade genjavadoc and use upstream version
Posted by jodersky <gi...@git.apache.org>.
Github user jodersky commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-214849927
cc @mengxr
@JoshRosen, is the maven build ok? I could only find references to genjavadoc in sbt.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14511][Build] Upgrade genjavadoc to lat...
Posted by mengxr <gi...@git.apache.org>.
Github user mengxr commented on the pull request:
https://github.com/apache/spark/pull/12707#issuecomment-215628995
+1 on merging this first and fixing remaining issues later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org