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.bate...@oracle.com
<mailto:alan.bate...@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.
