<AWT Dev> [9] Review Request: 8047336 Read flavormap.properties as resource

[Anthony Petrov]

Thanks. Note that your email editor messed up the HTML part of the email 
(see below a text-only quote), so here's the correct link to the webrev:

http://cr.openjdk.java.net/~pchelko/9/8047336/webrev.01/

The fix looks fine to me.

--
best regards,
Anthony

On 7/2/2014 3:10 PM, Petr Pchelko wrote:
> Hello, Anthony, Alan.
>
> Thank you for the review, the new version is located here:
> http://cr.openjdk.java.net/~pchelko/9/8047336/webrev.01/
> <http://cr.openjdk.java.net/%7Epchelko/9/8047336/webrev.00/>
>
> I've changed the code to throw an InternalError if we cannot read
> properties.
> Once I get feedback from the build team - I'll push this fix.
>
> With best regards. Petr.
>
> On 02 июля 2014 г., at 13:52, Alan Bateman <Alan.Bateman at oracle.com
> <mailto:Alan.Bateman at oracle.com>> wrote:
>
>> On 01/07/2014 09:35, Petr Pchelko wrote:
>>> Hello,
>>>
>>> The changes in the public API have been approved, so let me continue
>>> the review process.
>>>
>>> For your convenience:
>>> The bug: https://bugs.openjdk.java.net/browse/JDK-8047336
>>> The fix: http://cr.openjdk.java.net/~pchelko/9/8047336/webrev.00/
>>> <http://cr.openjdk.java.net/%7Epchelko/9/8047336/webrev.00/>
>>>
>>> Until now we've been discussing an abstract question of moving the
>>> properties to a different location and agreed that it's possible.
>>> Now it's time for an official code review. Could someone please make one?
>>>
>>> Also some feedback from the build team is still required. Could
>>> someone from the build team review this fix please?
>>> (The question is that I've made a separate explicit rule for
>>> flavormap.properties file. If I add it to COPY_PATTERNS than the
>>> solaris version get's copied on Mac. Is that a bug in the build
>>> system? Is my current solution good enough?)
>>>
>> I think this looks okay. I see Anthony's mail asking about the warning
>> message and whether it should the print stack trace. I just wonder if
>> it might be saner to just throw InternalError here. It throws
>> InternalError if flavormap.properties is missing and it might be
>> consistent to do the same when the file is corrupt or can't be parsed
>> as a properties file.
>>
>> -Alan.
>