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

[Priya Lakshmi Muthuswamy]

Sure Jon, I will try to do that in a separate bug.

Thanks,
Priya

On 3/30/2019 12:16 AM, Jonathan Gibbons wrote:
>
> OK.
>
>
> Follow up discussion ...
>
> "grep" says that now that the frame files are going away, 
> AbstractPackageIndexWriter only has a single subtype, 
> PackageIndexWriter. Likewise AbstractModuleIndexWriter is only 
> subtyped by ModuleIndexWriter.  In both cases, it might be reasonable 
> to coalesce the Abstract* type and the subtype, thus eliminating the 
> need for the Abstract* type.  You should be able to do this with an 
> IDE, although you might need to carefully examine the results, 
> especially with regard to the ordering that might be chosen by the IDE.
>
> -- Jon
>
> On 03/29/2019 01:52 AM, Priya Lakshmi Muthuswamy wrote:
>>
>> 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/ 
>> <http://cr.openjdk.java.net/%7Epmuthuswamy/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/20190401/57c82d99/attachment.html>