8177511: RIP old doclet

[Kumar Srinivasan]

http://hg.openjdk.java.net/jdk10/jdk10/langtools/rev/ead6d6c18bcc

Kumar


> webrev.delta looks good.
>
> -- Jon
>
> On 06/21/2017 02:30 PM, Kumar Srinivasan wrote:
>>
>> Hi Jon,
>>
>> I noticed while I did a personal review there was one test being ignored
>> test/tools/javadoc/MaxWarns.java, provided a test doclet to simulate the
>> behavior of the html doclet, thatthe test expects.
>>
>> Besides the above I made some minor cosmetic changes.
>> Shown here in the delta webrev:
>> http://cr.openjdk.java.net/~ksrini/8177511/webrev-03/webrev/webrev.delta/
>>
>> Please use this patch for comparison purposes:
>> http://cr.openjdk.java.net/~ksrini/8177511/webrev-03/a8e4d2286eba.patch
>>
>> The mercurial tip is at: a8e4d2286eba
>>
>> Thanks
>> Kumar
>>
>>
>>> OK,
>>>
>>> typo in a test .. "implentations"
>>>
>>> suggest clean up comment in Start re: old tool and old doclet's Start
>>>
>>> suggest for future: deprecate old entry point for removal.
>>>
>>> (no need for another big webrev.)
>>>
>>> -- Jon
>>>
>>>
>>> On 06/19/2017 01:51 PM, Kumar Srinivasan wrote:
>>>>
>>>> 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
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20170622/b34810c9/attachment.html>