RFR : JDK-8215599 : Remove support for javadoc "frames" mode

[Priya Lakshmi Muthuswamy]

Hi Jon,

Thanks for the review.

In AbstractModuleIndexWriter.java, removed the empty list for 
addModulePackagesIndexContents method but missed the addIndexContents.
Updated the webrev with the suggestions.

updated webrev : http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.02/

Thanks,
Priya

On 3/29/2019 3:09 AM, Jonathan Gibbons wrote:
>
> Priya,
>
> Nearly there: minor comments below:
>
> In this file:
>
> http://cr.openjdk.java.net/%7Epmuthuswamy/8215599/webrev.01/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractModuleIndexWriter.java.sdiff.html
>
> you have not removed the empty list that I reported in the previous 
> review.   Look at lines 223-224 in the new code.
>
> http://cr.openjdk.java.net/%7Epmuthuswamy/8215599/webrev.01/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractPackageIndexWriter.java.sdiff.html
>
> line 112, You've deleted the @param documentation, but not the actual 
> parameter in the signature!
>
> General comment: IndexRedirectWriter: this class needs a better 
> explanatory comment. I figured out why we still need it, but it took a 
> while. I'll file a separate bug to improve the comment.
>
> ModuleIndexWriter, PackageIndexWriter
> I think the main comment may be wrong ... the filename is always 
> "index.html", isn't it, not "overview-summary.html"
>
> test/langtools/jdk/javadoc/doclet/WindowTitles/WindowTitles.java.
> line 57 .... previously, the arguments were vertically aligned. The 
> edit broke the alignment.
>
> -- Jon
>
>
> On 03/15/2019 02:25 AM, Priya Lakshmi Muthuswamy wrote:
>>
>> Hi Jon,
>>
>> I have updated the webrev.
>> http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.01/
>>
>> changeset id of the tip - 54142:6ab293f66cae
>>
>> Thanks,
>> Priya
>>
>> On 3/15/2019 4:33 AM, Jonathan Gibbons wrote:
>>>
>>> Priya,
>>>
>>> I'm trying to look at these changes with an IDE, which requires a 
>>> repo.  The patch in the webrev does not apply cleanly to the current 
>>> tip of the jdk/jdk repo.
>>>
>>> Can you either post an updated webrev, or just post the changeset id 
>>> of the rip of your repo, so that I can "hg update" my repo so that 
>>> the patch will apply cleanly.
>>>
>>> -- Jon
>>>
>>>
>>> On 03/11/2019 06:28 AM, Priya Lakshmi Muthuswamy wrote:
>>>>
>>>> Hi Jon,
>>>>
>>>> Thanks for the review.
>>>> I have updated the webrev.
>>>>
>>>> webrev : http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.01/
>>>>
>>>> Reg ModuleIndexWriter, old lines 98-106:
>>>> After removing frames, the methods AllClassesLink/AllModulesLink 
>>>> have been removed as they implemented in frame classes.
>>>> With out these, we are just creating empty list. That's the reason 
>>>> for deleting these lines.
>>>> This change is similar to AbstractModuleIndexWriter lines old lines 
>>>> 251-258, in the updated webrev, i have removed the creation of 
>>>> empty list.
>>>>
>>>> Thanks,
>>>> Priya
>>>>
>>>> On 3/9/2019 6:48 AM, Jonathan Gibbons wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 03/08/2019 03:12 AM, Priya Lakshmi Muthuswamy wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Kindly review the changes for removal of frames.
>>>>>> Removed the script for setting window title which was needed for 
>>>>>> frames and also the boolean includeScript,
>>>>>> which was false for overview-frame,allclasses-frame and 
>>>>>> package-frame and true for all other cases.
>>>>>>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8215599
>>>>>> webrev: http://cr.openjdk.java.net/~pmuthuswamy/8215599/webrev.00/
>>>>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8215603
>>>>>>
>>>>>> Thanks,
>>>>>> Priya
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> AbstractModuleIndexWriter.java
>>>>> lines 243,244
>>>>> You're creating an empty list and adding it to the header.
>>>>> I think you can delete these lines.
>>>>>
>>>>> AbstractPackageIndexWriter.java
>>>>> line 43:  bad grammar in comment "sub-classed by to"
>>>>>
>>>>>
>>>>> IndexRedirectWriter
>>>>> line 81: it's an unrelated change, but looks OK
>>>>>
>>>>>
>>>>> ModuleIndexWriter
>>>>> old lines 98-106: please explain why you have deleted these lines
>>>>> they don't look immediately related to the Frames support.
>>>>>
>>>>> standard.properties
>>>>> The message needs fixing. The user has asked for "no frames"
>>>>> and the message is about "frames": i.e. it's not directly related
>>>>> to what the user did. I suggest:
>>>>> 445 doclet.NoFrames_specified=\
>>>>> 446 The --no-frames option is no longer required and may be removed\n
>>>>> 447 in a future release.
>>>>>
>>>>> DocPaths.java
>>>>> I'm slightly surprised there were quite so many variants of 
>>>>> allclasses*.html files!
>>>>>
>>>>> TestGeneratedBY.java
>>>>> You've commented out a test ...
>>>>>
>>>>> -- Jon
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20190329/ff25f6a6/attachment-0001.html>