<Swing Dev> Review Request for 8040893: fix doclint block tag issues in swing border classes

Alexander Scherbatiy alexandr.scherbatiy at oracle.com
Thu Apr 24 08:36:47 UTC 2014


  The fix looks good for me.

  Thanks,
  Alexandr.


On 4/23/2014 9:49 PM, Steve Sides wrote:
> this has the noted change for getEtchType():
>
> http://cr.openjdk.java.net/~ssides/8040893/8040893.3/
>
> -steve
>
> On 4/22/2014 2:03 AM, Alexander Scherbatiy wrote:
>> On 4/21/2014 11:54 PM, Steve Sides wrote:
>>>
>>> On 4/21/2014 2:45 AM, Alexander Scherbatiy wrote:
>>>>
>>>> Hello Steve,
>>>>
>>>> ------------------------
>>>>      /**
>>>>       * Returns the type of the bevel border.
>>>> +     *
>>>> +     * @return 0 if the bevel type is {@code RAISED}, 1 if {@code 
>>>> LOWERED}
>>>>       */
>>>> ------------------------
>>>>
>>>> I think such description reveals unnecessary level of code 
>>>> abstraction.
>>>> Developers always need to use BevelBorder.RAISED and 
>>>> BevelBorder.LOWERED constant and never their numerical representation.
>>>> It would better to say that the method return the bevel type: 
>>>> RAISED or  LOWERED.
>>> I thought about that, but the method signature is
>>> public *int*getBevelType()
>>> and I wasn't sure if saying it returns "RAISED or LOWERED" clearly 
>>> matched the signature. I will make it so.
>>
>>    This looks good.
>>    There is the javadoc for the 
>> BorderLayout.addLayoutComponent(Component comp, Object constraints) 
>> method that says:
>>    "For border layouts, the constraint must be one of the following 
>> constants: NORTH, SOUTH, EAST, WEST, or CENTER."
>>    and does not mention that NORTH constant is actually the "North" 
>> String.
>> http://docs.oracle.com/javase/7/docs/api/java/awt/BorderLayout.html
>>
>>   Could you also update the javadoc for the 
>> EtchedBorder.getEtchType() method in the same way?
>>
>>>
>>>>
>>>> ------------------------
>>>>      /**
>>>>       * Returns whether or not the border is opaque.
>>>> +     *
>>>> +     * @return true
>>>>       */
>>>>      public boolean isBorderOpaque() { return true; }
>>>> ------------------------
>>>> It would better to also add the similar comment from the parent 
>>>> AbstractBorder class like: "This implementation returns true".
>>>> It seems that the "code" tag can be used with boolean values: 
>>>> {@code true}
>>>
>>> okay, http://cr.openjdk.java.net/~ssides/8040893/8040893.2/ has 
>>> changes for above
>>
>>    This version looks good for me.
>>
>>   Thanks,
>>   Alexandr.
>>>
>>> thanks for the quick feedback,
>>>
>>> -steve
>>>
>>>>
>>>> Thanks,
>>>> Alexandr.
>>>>
>>>> On 4/18/2014 2:48 AM, Steve Sides wrote:
>>>>> Hello,
>>>>>
>>>>> Could you please review the fix for the following bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8040893
>>>>>
>>>>> Webrev corresponding:
>>>>> http://cr.openjdk.java.net/~ssides/8040893/
>>>>>
>>>>> webrevComment.txt:
>>>>> This addresses missing @parm and @return block tags in javadoc for
>>>>> javax/swing/border classes as noted by doclint.
>>>>>
>>>>> It does not address methods which are missing javadoc comment 
>>>>> altogether,
>>>>> of which there are many.
>>>>>
>>>>> It also a adds @return to isBorderOpaque() methods which were 
>>>>> incorrectly
>>>>> inheriting the default, @return false, from AbstractBorder.
>>>>>
>>>>>
>>>>> thanks,
>>>>>
>>>>> -steve
>>>>>
>>>>
>>>
>>
>




More information about the swing-dev mailing list