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

Petr Pchelko petr.pchelko at oracle.com
Thu Jul 10 06:55:00 UTC 2014


Hello, Build Team.

This is a reminder. Could you please review the build part of the following fix:
http://cr.openjdk.java.net/~pchelko/9/8047336/webrev.01/

Thank you.
With best regards. Petr.

On 02 июля 2014 г., at 15:13, Anthony Petrov <anthony.petrov at oracle.com> wrote:

> 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.
>> 




More information about the build-dev mailing list