Peter,
I can hold off on the size() changes all together.
The problem is with other bug fixes is the time it takes for the roundtrip
of the review process, the 3 hours time difference, and the weekend that
is coming in.
If any of you is not comfortable with any of our fixes, feel free to say so
(this is exactly why I asked first, *before* making substantial changes).
Shelly is not running on any intermediate builds, only nightlies.
thanks,
-marina
Peter Krogh wrote:
> I have the same concern about all the non cts bugs that are being fixed right
> now.
>
> Shelly is running CTS based on a drop we are getting in today (or early
> tomorrow). How about if these bugs get held off until that run is started?
>
> -----Original Message----- From: Marina Vatkina
> [mailto:Marina.Vatkina_at_Sun.COM] Sent: Friday, March 17, 2006 4:49 PM To:
> persistence_at_glassfish.dev.java.net Subject: Re: Issue 401: Moving size()
> outside a for loop?
>
>
> Hello Gordon,
>
> Yes, I know. This is why I asked. One the other hand, is it worth trying?
> After I fix hints/property names to be toplink..., I can make the changes and
> we can run all the tests (CTS, perf, unit, etc.). If it causes at least 1
> regression, I'll attach the changes to the bug report, and we'll decide what
> to do with this later.
>
> Would you agree?
>
> thanks, -marina
>
>
> Gordon Yorke wrote:
>
>> Hello Marina, I think changing 63 files at this point (so close to our CTS
>> goal) could be problematic? --Gordon
>>
>> -----Original Message----- From: Marina Vatkina
>> [mailto:Marina.Vatkina_at_Sun.COM] Sent: Friday, March 17, 2006 4:39 PM To:
>> persistence Subject: Issue 401: Moving size() outside a for loop?
>>
>>
>> Peter, Tom,
>>
>> There are 63 files where I found size() in the for loop (using grep, so
>> obviously I need to go over each case to double check).
>>
>> Do you see any problem with moving all of them out of the for loop?
>>
>> I also saw (at least once) where the code "manually" creates an Object[]
>> from a collection. Do you see a problem if I replace it with toArray()
>> call?
>>
>> While it's a lot of changes, I can make them before Monday so that we can
>> try to run them with performance benchmarks and see if it makes a
>> difference.
>>
>> What do you think?
>>
>> thanks, -marina