Ok here are the new changes, also please see my inlined responses below.
http://cr.openjdk.java.net/~ksrini/8177511/webrev-02/
Full webrev
http://cr.openjdk.java.net/~ksrini/8177511/webrev-02/webrev
Delta webrev
http://cr.openjdk.java.net/~ksrini/8177511/webrev-02/webrev.delta/
Please use this patch directly generated by hg, the patch generated by
webrev wrt. the directories containing removed binaries especially
images.
http://cr.openjdk.java.net/~ksrini/8177511/webrev-02/8177511.patch
(Completed this round of review.)
-- Jon
These review comments are based on a meld comparison, derived from
the webrev langtools.patch.
# Comparing src directories
/w/jjg/work/reviews/jdk10.remove.doclet/langtools/src/jdk.javadoc/share/classes/com/sun/tools/doclets/standard/Standard.java
line 42, remove debug print.
Fixed
Suggest leaving in
/**
* This is not the doclet you are looking for.
* @deprecated The doclet has been superseded by its replacement,
* {@code jdk.javadoc.doclets.StandardDoclet}.
*/
@Deprecated(forRemoval=true, since="9")
Fixed
There is stuff left in
com.sun.tools.doclets.internal.toolkit.resources that has not been
deleted. Please ensure that all these files are gone. Most of the
cruft seems to be image files. Try doing
hg remove --after
src/jdk.javadoc/share/classes/com/sun/tools/doclets/internal
No it is patch generated by webrev please see explanation above
/w/jjg/work/reviews/jdk10.remove.doclet/langtools.ref/src/jdk.javadoc/share/classes/com/sun/tools/doclets/standard/package-info.java
line 28, suggest replacing "superseded" by "replaced".
Fixed
/w/jjg/work/reviews/jdk10.remove.doclet/langtools.ref/src/jdk.javadoc/share/classes/com/sun/tools/javadoc/main/DocImpl.java
change from FatalError to Error looks suspicious. That being said,
I don't see any code to catch/handle FatalError so maybe this is OK.
yes should be ok.
Start.java
Why has hasOldTaglet not gone away?
Why has OldStdDocletName not gone away?
Why has "Step 5" (lines 769-773) not gone away
Fixed, also eradicated -Xold.
# Comparing test directories
I see some undeleted jar files. Is that just a webrev/meld artifact
like the images in src/ ?
Webrev/hg I think, please use the above patch.
/w/jjg/work/reviews/jdk10.remove.doclet/langtools.ref/test/tools/javadoc/6942366/T6942366.java
If you add @ignore, it should be followed by a bug number and synopsis.
Alternative is to provide/use a new toy doclet that reports included
classes, or else just delete the test.
Fixed it, I think the test can be improved perhaps in the new doclet.
/w/jjg/work/reviews/jdk10.remove.doclet/langtools.ref/test/tools/javadoc/6964914/Test.java
Not sure why you've had to edit this.
Ah, note the test is invoking the std doclet with doclint, but
doclint is invoked by the doclet
so removed the error and option which is no longer recognized by the
*silly* doclet.
/w/jjg/work/reviews/jdk10.remove.doclet/langtools.ref/test/tools/javadoc/badSuper/BadSuper.java
Delete the test
Done.
/w/jjg/work/reviews/jdk10.remove.doclet/langtools/test/tools/javadoc/lib/ToyDoclet.java
Would it be useful to have in the new world as well? (Separate RFE,
I guess)
Yep
/w/jjg/work/reviews/jdk10.remove.doclet/langtools/test/tools/javadoc/parser/7091528/T7091528.java
hmmm, OK; it wasn't a very good test to begin with, and now it's
even less good. But OK.
Fixed it
Thanks
Kumar
