[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-swing-dev
Subject: Re: <Swing Dev> JDK 9 RFR of JDK-8075082: Fix missing doclint warnings in the javax.swing package
From: "Anton V. Tarasov" <anton.tarasov () oracle ! com>
Date: 2015-04-16 12:37:03
Message-ID: 552FACEF.10605 () oracle ! com
[Download RAW message or body]
Hi Joe,
On 15.04.2015 20:02, joe darcy wrote:
> Hi Anton,
>
> On 4/15/2015 8:14 AM, Anton V. Tarasov wrote:
>> Hi Joe,
>>
>> (Looking into DefaultFocusManager.java)
>>
>> Formally, the @return description is not good here:
>> + /**
>> + * Compare the components.
>> + * @param a the first component
>> + * @param b the second component
>> + * @return the result of comparing the components
>> + */
>> public boolean compareTabOrder(Component a, Component b) {
>> return (comparator.compare(a, b) < 0);
>> }
>> because not the components are compared for equality but their order in a focus traversal cycle.
>
> Okay; I'll push the change with the more accurate comment:
>
> /**
> * Compares the components by their focus traversal cycle order.
> * @param a the first component
> * @param b the second component
> * @return a comparison of the components by their focus traversal cycle order
> */
Looks good.
>
>>
>> (Also, just a remark. The swing's FocusManager and DefaultFocusManager classes are obsolete since
>> JDK 1.4, they're not used any longer actually.)
>
> I won't tackle the issue myself, but it sounds like marking such classes as obsolete / deprecated
> would be a good cleanup for later in JDK 9.
Sure, I just wanted to note that those classes are of no worth (and I see you're fixing the
warnings, so my remark was just a formality). Also, there's a note in the FocusManager's javadoc
description about its obsoleteness.
Thanks,
Anton.
>
> Thanks for the review,
>
> -Joe
>
>>
>> Regards,
>> Anton.
>>
>> On 14.04.2015 21:02, Sergey Bylokhov wrote:
>>> HI, Joe.
>>> The fix looks good. But it will be good if Anton(CC) will take a look to the documentation of
>>> DefaultFocusManager.java
>>>
>>> On 10.04.15 4:22, joe darcy wrote:
>>>> ping
>>>>
>>>> Any comments or objections to this fix?
>>>>
>>>> -Joe
>>>>
>>>> On 4/7/2015 2:32 PM, joe darcy wrote:
>>>>> Hello,
>>>>>
>>>>> Please review changes to address
>>>>>
>>>>> JDK-8075082: Fix missing doclint warnings in the javax.swing package
>>>>> http://cr.openjdk.java.net/~darcy/8075082.0/
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Joe
>>>>>
>>>>
>>>
>>>
>>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi Joe,<br>
<br>
On 15.04.2015 20:02, joe darcy wrote:<br>
</div>
<blockquote cite="mid:552E99B3.4010003@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
Hi Anton,<br>
<br>
<div class="moz-cite-prefix">On 4/15/2015 8:14 AM, Anton V.
Tarasov wrote:<br>
</div>
<blockquote cite="mid:552E8064.4020004@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Joe,<br>
<br>
(Looking into DefaultFocusManager.java)<br>
<br>
Formally, the @return description is not good here:<br>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant: normal; \
font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; \
text-align: start; text-indent: 0px; text-transform: none; widows: auto; \
word-spacing: 0px; -webkit-text-stroke-width: 0px;"><span class="new" style="color: \
blue; font-weight: normal;">+ /**</span> <span class="new" style="color: blue; \
font-weight: normal;">+ * Compare the components.</span> <span class="new" \
style="color: blue; font-weight: normal;">+ * @param a the first component</span> \
<span class="new" style="color: blue; font-weight: normal;">+ * @param b the \
second component</span> <span class="new" style="color: blue; font-weight: normal;">+ \
* @return the result of comparing the components</span> <span class="new" \
style="color: blue; font-weight: normal;">+ */</span> public boolean \
compareTabOrder(Component a, Component b) { return (comparator.compare(a, b) < \
0); }</pre>
because not the components are compared for equality but their
order in a focus traversal cycle.<br>
</div>
</blockquote>
<br>
Okay; I'll push the change with the more accurate comment:<br>
<br>
/**<br>
* Compares the components by their focus traversal cycle
order.<br>
* @param a the first component<br>
* @param b the second component<br>
* @return a comparison of the components by their focus
traversal cycle order<br>
*/<br>
</blockquote>
<br>
Looks good.<br>
<br>
<blockquote cite="mid:552E99B3.4010003@oracle.com" type="cite"> <br>
<blockquote cite="mid:552E8064.4020004@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
(Also, just a remark. The swing's FocusManager and
DefaultFocusManager classes are obsolete since JDK 1.4,
they're not used any longer actually.)<br>
</div>
</blockquote>
<br>
I won't tackle the issue myself, but it sounds like marking such
classes as obsolete / deprecated would be a good cleanup for later
in JDK 9.<br>
</blockquote>
<br>
Sure, I just wanted to note that those classes are of no worth (and
I see you're fixing the warnings, so my remark was just a
formality). Also, there's a note in the FocusManager's javadoc
description about its obsoleteness.<br>
<br>
Thanks,<br>
Anton.<br>
<br>
<blockquote cite="mid:552E99B3.4010003@oracle.com" type="cite"> <br>
Thanks for the review,<br>
<br>
-Joe<br>
<br>
<blockquote cite="mid:552E8064.4020004@oracle.com" type="cite">
<div class="moz-cite-prefix"> <br>
Regards,<br>
Anton.<br>
<br>
On 14.04.2015 21:02, Sergey Bylokhov wrote:<br>
</div>
<blockquote cite="mid:552D5629.6070500@oracle.com" type="cite">HI,
Joe. <br>
The fix looks good. But it will be good if Anton(CC) will take
a look to the documentation of DefaultFocusManager.java <br>
<br>
On 10.04.15 4:22, joe darcy wrote: <br>
<blockquote type="cite">ping <br>
<br>
Any comments or objections to this fix? <br>
<br>
-Joe <br>
<br>
On 4/7/2015 2:32 PM, joe darcy wrote: <br>
<blockquote type="cite">Hello, <br>
<br>
Please review changes to address <br>
<br>
JDK-8075082: Fix missing doclint warnings in the
javax.swing package <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edarcy/8075082.0/">http://cr.openjdk.java.net/~darcy/8075082.0/</a>
<br>
<br>
Thanks, <br>
<br>
-Joe <br>
<br>
</blockquote>
<br>
</blockquote>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic