[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    [OpenJDK 2D-Dev] [PATCH] SurfaceManagerFactory
From:       Dmitri.Trembovetski () Sun ! COM (Dmitri Trembovetski)
Date:       2008-05-14 23:22:01
Message-ID: 482B7419.7040005 () Sun ! COM
[Download RAW message or body]


   Hi Roman,

   OK, I have finally integrated this fix (with
   a couple of minor changes after internal
   code review):
     http://hg.openjdk.java.net/jdk7/2d/jdk/rev/4af4867ed787

   From the code review by Chris (should have CC-ed you in the
   first place):
 >> SMF.java
 >> 49: Take out the "Unix" part.
 >
 >   Fixed.
 >
 >> 50,51: I don't think that's what it does.
 >
 >   Removed the comment. If this is added later,
 >   will add back in.
 >
 >> 56, 61, 72, 84: Unnecessary blank lines.
 >
 >   Removed.
 >
 >> 66: {@code null}
 >
 >   Fixed.
 >
 >> 75: I'd prefer an IllegalArgumentException with an explanatory message
 >> (e.g. "factory must be non-null").
 >
 >   Fixed.

   Thanks,
     Dmitri

Dmitri Trembovetski wrote:
> 
>   Hi Roman,
> 
> Roman Kennke wrote:
>>>    I can tell that someone didn't build on windows
>>>    since the build fails there =) The fix should have deleted
>>>    src/windows/classes/sun/java2d/SurfaceManagerFactory.java
>>>    just like it did the solaris/ one.
>>
>> Whoops, did I forget that? I was sure I deleted it locally, so somehow
>> it didn't make it into the patch. Sorry for that.
> 
>   No problem, I should have spotted it during review anyway.
>   I'm just not used to reviewing by looking at the raw diff
>   output, we use that webrev tool for generating html for
>   code reviews, much easier on the eyes.
> 
>>>    In general, if the fix touches shared code it is
>>>    very advisable to build on all platforms - at
>>>    least on 32-bit solaris/linux/windows.
>>
>> The problem is, I don't have a Windows box (easily) available. I will
>> have to setup one at work, including all the build machinery for
>> OpenJDK.
> 
>   I myself use vmware to compile on windows if needed- then you'd only need
>   a windows license (and the compilers - although someone did compile
>   the jdk with the free VS2008, I believe). Of course, if you'll be 
> touching
>   any hw-acceleration related stuff, you'll need a real box to test on,
>   so you may as well get the box.
> 
>   Thanks,
>     Dmitri
> 


[prev in list] [next in list] [prev in thread] [next in thread]