Now it looks OK for CCC request:
http://ccc.us.oracle.com/docs/process
Thanks,
SAM
On 30.04.2014 2:54, Steve Sides wrote:
Hopefully 4th time's a charm.
Review Request for 8040893: fix doclint block tag issues in swing
border classes, rev 4:
http://cr.openjdk.java.net/~ssides/8040893/8040893.4/
-steve
On 4/25/2014 9:22 AM, sergey malenkov wrote:
On 25.04.2014 20:03, sergey malenkov wrote:
Hi Steve,
We should not confuse the cause and the result.
The getShadowOuterColor method is a property of this Border object.
Actually this object does not know about the component.
So, we can say here that the color will be derived from background.
!!! can't say
The getShadowOuterColor(Component) method is an utility method,
that provides all calculations to resolve the null value of the
property.
Here we specify that the *somehow modified* background color
will be used if the shadowOuterColor property is not set.
Thanks,
SAM
On 24.04.2014 20:41, Steve Sides wrote:
Hi Sergey,
I see what you mean. I have a question on some of the BevelBorder
method docs.
Regarding the getHighlight(or Shadow)Inner/Outer)Color() methods
(no parameter)
http://cr.openjdk.java.net/~ssides/8040893/8040893.3/src/share/classes/javax/swing/border/BevelBorder.java.sdiff.html
For the methods which take a Component parameter, it is commented
the color is derived from the Component's background, so these
always return some Color.
For the methods which take no Component parameter, they return null
if the color is derived from the background(ie., not specified at
instantiation).
My question is, are comments like
201 * @return the outer highlight {@code Color} or {@code
null} if highlight
202 * colors are derived from background
seen as referring too much to the implementation or clarifying
statements?
If it simply says "...or {@code null} is none was specified", it
leaves it unclear if there is a highlight or shadow color.
It seems that the javadoc comment could have said
* Returns the outer highlight color of the bevel border.
* If none was specified at instantiation, the highlight
* color is derived from the background.
Then the return could have said, "or null if none was specified"
and there would be more clarity.
Or, do we not want that kind of implementation detail revealed at all?
-steve
On 4/24/2014 4:39 AM, sergey malenkov wrote:
Hello,
I don't think that it is a good idea to specify current
implementation.
For example,
* Returns whether or not the border is opaque.
+ *
+ * @return true if {@code color} is not {@code null}, false
if {@code color}
+ * is {@code null}
Once it is specified we never can fix it.
For example, the MatteBorder.isBorderOpaque method, specified above,
can be fixed in the following way:
+ * @return true if the internal color is not null *and uses
alpha-channel*.
So I suggest to rewrite it's documentation in the following way:
* Returns whether or not the border is opaque.
+ *
+ * @return {@code true} if this border is opaque,
+ * {@code false} otherwise
The same issue with other similar methods.
Thanks,
SAM
On 23.04.2014 21:49, 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
