<AWT Dev> [PATCH] Cleanup AWT peer interfaces

[Artem Ananiev]

Roman Kennke wrote:
> Hi Artem,
> 
>>> Cool! I'm comfortable with this fix now.
>>>
>>> We still need second approve vote to push this change in.
>>> Artem...? Others? ;)
>> Well, I'm fine with the proposed changes in general. Still, some JavaDoc 
>> comments looks to be too implementation-specific. For example, most of 
>> Container's methods contain a statement like this:
>>
>> This is called from {@link ...}, before/after ...
>>
>> and they really called from the specified places, however this may not 
>> be true in future. I'd remove all these statements from JavaDoc, leaving 
>> only behavioral description.
> 
> Yeah, you are right. I put these in because I found it very useful while
> implementing the peers to see exactly where and when it is called. I
> removed those comments and only let the @see references in there. The
> updated webrev is here:
> 
> http://kennke.org/~roman/docpeers/webrev/
> 
> Ok now?

Fine, thank you.

Artem

> Cheers, Roman
> 
>> Thanks,
>>
>> Artem
>>
>>> Thanks,
>>>   Andrei
>>>
>>> Roman Kennke wrote:
>>>> Hi Andrei,
>>>>
>>>> Finally I came around to fix the suggested stuff. See comments inline.
>>>>
>>>>> 1) src/share/classes/java/awt/peer/CheckboxMenuItemPeer.java
>>>>>
>>>>> +import java.awt.CheckboxMenuItem;
>>>>> if that's really needed? I thought that {@link ...} doesn't require 
>>>>> such stuff.
>>>> I don't know. If you fully qualify the stuff in the {@link } it's not
>>>> needed. Should I fully-qualify things everywhere or let the import in
>>>> place? I don't care really.
>>>>
>>>>
>>>>> 2) src/share/classes/java/awt/peer/ContainerPeer.java
>>>>> There is a typo in the second word:
>>>>> -     * Indicates availabiltity of restacking operation in this 
>>>>> container.
>>>>> +     * Indicates availability of restacking operation in this 
>>>>> container.
>>>> Fixed.
>>>>
>>>>> 3) Common thing.
>>>>> <code>true</code> have a shorter synonym since JDK 5: {@code true}
>>>> Fixed.
>>>>
>>>>> 4) src/share/classes/java/awt/peer/RobotPeer.java
>>>>> I noticed that you prefer not to leave "public" modifier in an 
>>>>> interface. But here you are leaving all of them.
>>>> Fixed.
>>>>
>>>>> 5) src/share/classes/java/awt/peer/RobotPeer.java
>>>>> public int getNumberOfButtons(); has left w/o any comments.
>>>> Fixed. I'm not sure if I got the sematics right.
>>>>
>>>>> 6) src/share/classes/java/awt/peer/ComponentPeer.java
>>>>> +    /**
>>>>> +     * Called by {@link EventQueue#coalescePaintEvent} to let the 
>>>>> component
>>>>> +     * peer coalesce paint events.
>>>>> +     *
>>>>> +     * @param e the paint event to consider to coalesce
>>>>> +     *
>>>>> +     * @see EventQueue#coalescePaintEvent
>>>>> +     */
>>>>> +    void coalescePaintEvent(PaintEvent e);
>>>>>
>>>>> I'd say it's not "to let .... coalesce paint events", but "to 
>>>>> coalesce paint events".
>>>> Fixed.
>>>>
>>>>> 7) at the same class.
>>>>> You wrote:
>>>>> +    // TODO: Maybe change this to force Graphics2D, since many 
>>>>> things will
>>>>> +    // break with plain Graphics nowadays.
>>>>> +    Graphics getGraphics();
>>>>>
>>>>> Do you know a scenario to show what's exactly might be broken. We 
>>>>> probably need to introduce another peer for that, right?
>>>> I already explained my reason in earlier mails. You think this is
>>>> reasonable?
>>>>
>>>>
>>>>> 8) Similar to 7):
>>>>> +    // TODO: Maybe make that return a BufferedImage, because some 
>>>>> stuff will
>>>>> +    // break if a different kind of image is returned.
>>>>> +    Image createImage(int width, int height);
>>>> Dito.
>>>>
>>>> This time I did the work on top of the -awt workspace and created a
>>>> webrev for your reviewing pleasure:
>>>>
>>>> http://kennke.org/~roman/docpeers/webrev/
>>>>
>>>> Thanks for having a look at it again,
>>>>
>>>> Roman
>>>>