<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