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

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] RFR: 8249821: Separate libharfbuzz from libfontmanager
From:       Erik Joelsson <erik.joelsson () oracle ! com>
Date:       2020-07-22 12:45:08
Message-ID: 66de937a-146a-202a-ad00-34b57a60d158 () oracle ! com
[Download RAW message or body]

Looks good, thanks!

/Erik

On 2020-07-21 15:18, Philip Race wrote:
> Hi,
>
> I noticed the indent problems at 434 & 436 myself once I looked at it 
> as a webrev,
> so I was already updating the fix with those but I've also fixed the 
> rest you pointed out.
> I've uploaded a new webrev with all of these resolved :-
>
> http://cr.openjdk.java.net/~prr/8249821.1
>
> -phil
>
> On 7/21/20, 3:03 PM, Erik Joelsson wrote:
>> Hello Phil,
>>
>> This looks pretty good, only some style nits.
>>
>> Awt2dLibraries.gmk
>>
>> 428 and 466: These comments aren't relevant anymore now that harfbuzz 
>> is its own library.
>>
>> 434: Missed indent of 2
>>
>> 436: Too much indent (should be 2)
>>
>> 459-464: Too much indent
>>
>> 493-494: Too much indent
>>
>> /Erik
>>
>> On 2020-07-21 14:39, Philip Race wrote:
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8249821
>>> Webrev: http://cr.openjdk.java.net/~prr/8249821/
>>>
>>> This fix breaks out libharfbuzz from libfontmanager.
>>> As well as building I've done extensive testing on all platforms.
>>>
>>> I have tweaked the disabled warnings so we don't have un-needed 
>>> duplication
>>> but it is not a goal of this fix to resolve them.
>>> There's a bug (JDK-8074844) already filed to resolve them for 
>>> fontmanager and
>>> that should be easier to fix now.
>>>
>>> -phil.
>>>
>>>
>>>
[prev in list] [next in list] [prev in thread] [next in thread]