You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Connor Doyle <co...@mesosphere.io> on 2015/06/27 03:18:48 UTC
Review Request 35961: Include protobuf classes in generated Javadoc.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/
-----------------------------------------------------------
Review request for mesos, Adam B and Ben Whitehead.
Bugs: MESOS-1552
https://issues.apache.org/jira/browse/MESOS-1552
Repository: mesos-incubating
Description
-------
Updates the Java POM to include the generated Java protobuf classes.
Diffs
-----
src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
src/java/mesos.pom.in 138da55
src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
Diff: https://reviews.apache.org/r/35961/diff/
Testing
-------
- configure
- make
- Manual verification of generated Javadoc HTML
Thanks,
Connor Doyle
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Ben Whitehead <be...@mesosphere.io>.
> On June 26, 2015, 6:30 p.m., Ben Whitehead wrote:
> > Ship It!
Downloaded patch, applied, configure make, Manual Verification.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89605
-----------------------------------------------------------
On June 26, 2015, 6:18 p.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 6:18 p.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Ben Whitehead <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89605
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Whitehead
On June 26, 2015, 6:18 p.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 6:18 p.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Adam B <ad...@mesosphere.io>.
> On July 8, 2015, 12:38 p.m., Ben Mahler wrote:
> > Any plan to get this onto the website?
Unfortunately, our website build/deploy Rakefile doesn't use the standard maven way to generate javadoc, and just runs the javadoc command manually. See https://issues.apache.org/jira/browse/MESOS-2959
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review90969
-----------------------------------------------------------
On June 29, 2015, 8:53 a.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 8:53 a.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review90969
-----------------------------------------------------------
Any plan to get this onto the website?
- Ben Mahler
On June 29, 2015, 3:53 p.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 3:53 p.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89839
-----------------------------------------------------------
Ship it!
Ship It!
- Adam B
On June 29, 2015, 8:53 a.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 8:53 a.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89753
-----------------------------------------------------------
Patch looks great!
Reviews applied: [35961]
All tests passed.
- Mesos ReviewBot
On June 29, 2015, 3:53 p.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 3:53 p.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Connor Doyle <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/
-----------------------------------------------------------
(Updated June 29, 2015, 3:53 p.m.)
Review request for mesos, Adam B and Ben Whitehead.
Changes
-------
Use `@abs_top_builddir@` instead of `${project.basedir}` in Maven POM template.
Bugs: MESOS-1552
https://issues.apache.org/jira/browse/MESOS-1552
Repository: mesos-incubating
Description
-------
Updates the Java POM to include the generated Java protobuf classes.
Diffs (updated)
-----
src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
src/java/mesos.pom.in 138da55
src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
Diff: https://reviews.apache.org/r/35961/diff/
Testing
-------
- configure
- make
- Manual verification of generated Javadoc HTML
Thanks,
Connor Doyle
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Connor Doyle <co...@mesosphere.io>.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in, lines 27-36
> > <https://reviews.apache.org/r/35961/diff/1/?file=993816#file993816line27>
> >
> > Did you verify that the curly-braces are properly handled here? I read on http://stackoverflow.com/a/13512524/4056606 that the `{@code` tag will interpret the first closing brace as its own ending brace. In fact, @code may only be necessary for wrapping Generic `<something>` declarations. You could additionally/instead use the old-style `<code>` tag to print everything in code-font. I would hope this would be easier, but parsing code within docs within code must be difficult. Don't even try to put `/*` block comments inside your code example, or the compiler will get confused too.
>
> Connor Doyle wrote:
> Yes, it does seem to work. Here is the output as rendered by my up-to-date Google Chrome browser on OS X:
>
> [](http://s15.postimg.org/p44aqh2e2/Screen_Shot_2015_06_29_at_07_47_24.jpg)
>
> However, I personally find the `<code></code>` markup more readable than the dangling closing curling brace. I will update this to use the plain HTML tag style.
The `<code></code>` variant causes the build to fail with Java 8. The javadoc parser chokes on the `>` symbol for some reason. I will verify that the `{@code ... }` style still works with Java < 8.
- Connor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
-----------------------------------------------------------
On June 27, 2015, 1:18 a.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 27, 2015, 1:18 a.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Ben Whitehead <be...@mesosphere.io>.
> On June 27, 2015, 11:59 a.m., Adam B wrote:
> > src/java/mesos.pom.in, line 134
> > <https://reviews.apache.org/r/35961/diff/1/?file=993817#file993817line134>
> >
> > Is `${project.basedir}/generated` any better/different than `@abs_top_builddir@/src/java/generated`
>
> Connor Doyle wrote:
> Probably not, will update to use `@abs_top_builddir@` instead.
>
> Connor Doyle wrote:
> Updated and verified.
`${project.basedir}/generated` is a maven interprited variable rather than a configure interprited variable, so it's evaluated based on the context in which maven is started. It would be convenient to try and keep the maven pom as close to vanilla maven as possible to reduce the complexities when dealing with that file. Perhaps this is something that can be cleaned up at a latter point.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
-----------------------------------------------------------
On June 29, 2015, 8:53 a.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 8:53 a.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Connor Doyle <co...@mesosphere.io>.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in, lines 27-36
> > <https://reviews.apache.org/r/35961/diff/1/?file=993816#file993816line27>
> >
> > Did you verify that the curly-braces are properly handled here? I read on http://stackoverflow.com/a/13512524/4056606 that the `{@code` tag will interpret the first closing brace as its own ending brace. In fact, @code may only be necessary for wrapping Generic `<something>` declarations. You could additionally/instead use the old-style `<code>` tag to print everything in code-font. I would hope this would be easier, but parsing code within docs within code must be difficult. Don't even try to put `/*` block comments inside your code example, or the compiler will get confused too.
Yes, it does seem to work. Here is the output as rendered by my up-to-date Google Chrome browser on OS X:
[](http://s15.postimg.org/p44aqh2e2/Screen_Shot_2015_06_29_at_07_47_24.jpg)
However, I personally find the `<code></code>` markup more readable than the dangling closing curling brace. I will update this to use the plain HTML tag style.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > src/java/mesos.pom.in, line 134
> > <https://reviews.apache.org/r/35961/diff/1/?file=993817#file993817line134>
> >
> > Is `${project.basedir}/generated` any better/different than `@abs_top_builddir@/src/java/generated`
Probably not, will update to use `@abs_top_builddir@` instead.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > src/java/src/org/apache/mesos/MesosExecutorDriver.java, line 31
> > <https://reviews.apache.org/r/35961/diff/1/?file=993818#file993818line31>
> >
> > Oracle docs (and wikipedia) claim that javadoc only uses the opening <p> tag, without the closing </p> tag. Did you find that this rendered incorrectly?
Good point. I ran the build with JAVA_HOME pointing to Java 8, which is much more stringent regarding Javadoc errors. Without this change, the build actually fails with Java 8. I will verify that this still works with Java < 8.
- Connor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
-----------------------------------------------------------
On June 27, 2015, 1:18 a.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 27, 2015, 1:18 a.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Connor Doyle <co...@mesosphere.io>.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > src/java/mesos.pom.in, line 134
> > <https://reviews.apache.org/r/35961/diff/1/?file=993817#file993817line134>
> >
> > Is `${project.basedir}/generated` any better/different than `@abs_top_builddir@/src/java/generated`
>
> Connor Doyle wrote:
> Probably not, will update to use `@abs_top_builddir@` instead.
Updated and verified.
- Connor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
-----------------------------------------------------------
On June 29, 2015, 3:53 p.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 3:53 p.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Connor Doyle <co...@mesosphere.io>.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > Thanks for doing this Connor! Just a couple of questions about javadoc parsing of html (I'm no expert, just googled a few things).
> > Do we need to do a pass over our protos now to convert any `//` or `/*` block comments to `/**` comments? Worth a follow-up JIRA/patch? Most of mesos.proto looks fine, except TrafficControlStatistics and ResourceStatistics. And I'm not sure how public these need to be, but messages.proto is all `//` style, or completely missing comments.
I think it makes sense to modify the comment style in a follow-up patch. As you mentioned the classes generated from mesos.proto already look pretty good.
- Connor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
-----------------------------------------------------------
On June 29, 2015, 3:53 p.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 29, 2015, 3:53 p.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Connor Doyle <co...@mesosphere.io>.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in, lines 27-36
> > <https://reviews.apache.org/r/35961/diff/1/?file=993816#file993816line27>
> >
> > Did you verify that the curly-braces are properly handled here? I read on http://stackoverflow.com/a/13512524/4056606 that the `{@code` tag will interpret the first closing brace as its own ending brace. In fact, @code may only be necessary for wrapping Generic `<something>` declarations. You could additionally/instead use the old-style `<code>` tag to print everything in code-font. I would hope this would be easier, but parsing code within docs within code must be difficult. Don't even try to put `/*` block comments inside your code example, or the compiler will get confused too.
>
> Connor Doyle wrote:
> Yes, it does seem to work. Here is the output as rendered by my up-to-date Google Chrome browser on OS X:
>
> [](http://s15.postimg.org/p44aqh2e2/Screen_Shot_2015_06_29_at_07_47_24.jpg)
>
> However, I personally find the `<code></code>` markup more readable than the dangling closing curling brace. I will update this to use the plain HTML tag style.
>
> Connor Doyle wrote:
> The `<code></code>` variant causes the build to fail with Java 8. The javadoc parser chokes on the `>` symbol for some reason. I will verify that the `{@code ... }` style still works with Java < 8.
Verified this works with Java version 1.7.0_55.
> On June 27, 2015, 6:59 p.m., Adam B wrote:
> > src/java/src/org/apache/mesos/MesosExecutorDriver.java, line 31
> > <https://reviews.apache.org/r/35961/diff/1/?file=993818#file993818line31>
> >
> > Oracle docs (and wikipedia) claim that javadoc only uses the opening <p> tag, without the closing </p> tag. Did you find that this rendered incorrectly?
>
> Connor Doyle wrote:
> Good point. I ran the build with JAVA_HOME pointing to Java 8, which is much more stringent regarding Javadoc errors. Without this change, the build actually fails with Java 8. I will verify that this still works with Java < 8.
Verified this works with Java version 1.7.0_55.
- Connor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
-----------------------------------------------------------
On June 27, 2015, 1:18 a.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 27, 2015, 1:18 a.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
-----------------------------------------------------------
Thanks for doing this Connor! Just a couple of questions about javadoc parsing of html (I'm no expert, just googled a few things).
Do we need to do a pass over our protos now to convert any `//` or `/*` block comments to `/**` comments? Worth a follow-up JIRA/patch? Most of mesos.proto looks fine, except TrafficControlStatistics and ResourceStatistics. And I'm not sure how public these need to be, but messages.proto is all `//` style, or completely missing comments.
src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in (lines 27 - 36)
<https://reviews.apache.org/r/35961/#comment142283>
Did you verify that the curly-braces are properly handled here? I read on http://stackoverflow.com/a/13512524/4056606 that the `{@code` tag will interpret the first closing brace as its own ending brace. In fact, @code may only be necessary for wrapping Generic `<something>` declarations. You could additionally/instead use the old-style `<code>` tag to print everything in code-font. I would hope this would be easier, but parsing code within docs within code must be difficult. Don't even try to put `/*` block comments inside your code example, or the compiler will get confused too.
src/java/mesos.pom.in (line 134)
<https://reviews.apache.org/r/35961/#comment142284>
Is `${project.basedir}/generated` any better/different than `@abs_top_builddir@/src/java/generated`
src/java/src/org/apache/mesos/MesosExecutorDriver.java (line 31)
<https://reviews.apache.org/r/35961/#comment142282>
Oracle docs (and wikipedia) claim that javadoc only uses the opening <p> tag, without the closing </p> tag. Did you find that this rendered incorrectly?
- Adam B
On June 26, 2015, 6:18 p.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 26, 2015, 6:18 p.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>
Re: Review Request 35961: Include protobuf classes in generated
Javadoc.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89643
-----------------------------------------------------------
Patch looks great!
Reviews applied: [35961]
All tests passed.
- Mesos ReviewBot
On June 27, 2015, 1:18 a.m., Connor Doyle wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35961/
> -----------------------------------------------------------
>
> (Updated June 27, 2015, 1:18 a.m.)
>
>
> Review request for mesos, Adam B and Ben Whitehead.
>
>
> Bugs: MESOS-1552
> https://issues.apache.org/jira/browse/MESOS-1552
>
>
> Repository: mesos-incubating
>
>
> Description
> -------
>
> Updates the Java POM to include the generated Java protobuf classes.
>
>
> Diffs
> -----
>
> src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
> src/java/mesos.pom.in 138da55
> src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
>
> Diff: https://reviews.apache.org/r/35961/diff/
>
>
> Testing
> -------
>
> - configure
> - make
> - Manual verification of generated Javadoc HTML
>
>
> Thanks,
>
> Connor Doyle
>
>