I like this patch!
Well, the only thing is with that:
public static boolean arebordersEqual(double border1, double border2) {
The name would look better if you follow the advice:
http://java.sun.com/docs/codeconv/html/CodeConventions.doc8.html#367
"Methods should be verbs, in mixed case with the first letter lowercase,
with the first letter of each internal word capitalized."
So, my vision is:
public static boolean areBordersEqual(double border1, double border2) {
Hey, am I behaving a bit strict? :) You don't need to send another patch, just push it.
Regards,
Andrei
Man Wong wrote:
Hi,
Sorry for the mis-spell, I will remember from now on :). And here is the
corrected patch, let me know when it gets approved. Thanks a lot.
Man Lung Wong
----- Original Message -----
From: "Andrei Dmitriev" <[email protected]>
To: "Man Wong" <[email protected]>
Cc: [email protected]
Sent: Tuesday, June 23, 2009 9:52:45 AM GMT -05:00 US/Canada Eastern
Subject: Re: <AWT Dev> Patch for LayoutExtraGaps
There are other occurrences of "boarders" in the file. I'd like all of
them also changed to be changed to "borders" or something like that if I
read their meaning correctly.
That bug number is for the reference only, you don't need to change it
either way. I do ;)
Could you please resubmit the patch?
Thanks,
Andrei
Man Wong wrote:
Hi,
Thanks for the reply. Should I make the fix and send it to you again? By the
way, for the fix, you only want the parameters name changed and should I also
change the bug information to the one that you just send me? Thanks again.
Man Lung Wong
----- Original Message -----
From: "Andrei Dmitriev" <[email protected]>
To: "Man Wong" <[email protected]>
Cc: [email protected]
Sent: Tuesday, June 23, 2009 7:12:39 AM GMT -05:00 US/Canada Eastern
Subject: Re: <AWT Dev> Patch for LayoutExtraGaps
Hi,
thanks for the analysis of this defect.
BTW, it's covered by the CR:
6848458: java/awt/GridLayout/LayoutExtraGaps/LayoutExtraGaps.java fails
(I can't find it through bugs.sun.com perhaps due to some temporary
problems with the site)
I like the fix but I'd name parameters not "boarders", but "borders"
otherwise it sounds like a variable receives some meal in a the method
scope in return for payment or service of holding the value in it :)
public static boolean areBoardersEqual(double boarder1, double boarder2) {
Thanks,
Andrei
Man Wong wrote:
Hi,
I was looking over one of the failures for a jtreg test on openjdk and the
test case did not make sense to me, which led me to make changes to the test
case.
First of all, the test was in java/awt/GridLayout/LayoutExtraGaps.java. The test
currently under the openjdk I believe was trying to test whether a GridLayout object
centre its component properly (based on the message printed by the exception). It tested
that by checking if the origin coordinate of the first component (each component is a
rectangle, and there are 29 rectangles in a panel and there are 4 panels in the main
window) is (0,0). If both x and y are 0 for any of the panels, then the test fails. I
also think that the reason why they chose (0,0) as the failing point because base on the
values they passed in, x and y cannot both be at (0,0). This is not valid because the
error that was output states, "Test failed. GridLayout doesn't center
component.", but the components are in fact centred, since all opposite boarders
have equal dimension.
When I looked at the gui generated, there are boarders between the
rectangles and its parent panel. And the boarder changes as the gui window
resizes (not sure if that is another problem in java or if it was intentional).
Not surprisingly, two of the panels were initialized such that there is no
boarder between itself and the rectangles, causing the test to fail. Which the
test should not have failed because everything was centred properly.
I created a fix to the test case (attached to this email) that checks if the boarder at the right equals the boarder at the left, and if the boarder at the top equals the boarder at the bottom. Instead of checking whether the origin coordinate of the first component is (0,0).
Thanks for looking things over, and hope to hear from you soon.
Man Lung Wong
