Hi Joe

Looks good

Best
Lance
On Jul 8, 2013, at 9:07 PM, huizhe wang wrote:

> Hi,
> 
> I've updated webrev with improved property management, and quality backed by 
> 320 test cases. Thanks Daniel for the help expanding the test suite!
> 
> Here's the webrev:
> http://cr.openjdk.java.net/~joehw/jdk8/8016648/webrev/
> 
> -Joe
> 
> On 7/5/2013 6:48 PM, huizhe wang wrote:
>> 
>> On 7/5/2013 1:32 AM, Alan Bateman wrote:
>>> On 04/07/2013 21:25, huizhe wang wrote:
>>>> 
>>>> Reverted back to the original code: 
>>>> http://cr.openjdk.java.net/~joehw/jdk8/8016648/webrev/
>>>> The code split the version number and look at the 1st or 2nd element, it 
>>>> thus works for both the current format and the proposed one, e.g. for 
>>>> 1.7.0, it compares with the 2nd element, and for the proposed 
>>>> MAJOR.MINOR.FU.*, the 1st or MAJOR.
>>> Thanks for dropping the dependency on javax.lang.model. What you now is 
>>> probably okay although I'm a bit dubious about attempting to support an 
>>> alternative format (I'm not aware of any proposal that changes the format 
>>> of the java.version property as changing it would like cause a lot of 
>>> breakage).
>> 
>> There was a survey from Iris last year, and the JPG site has a presentation 
>> from Aurelio. But you're right, I'll remove it. If there's any change in the 
>> future, that is if it happens at all, we can always add that back.
>>> 
>>> A minor point but isJDKOrAbove looks a bit odd to me, I'd probably go for 
>>> something like isJavaVersionGTE or isJavaVersionAtLeast but as it's not 
>>> part of the API then it doesn't matter of course.
>> 
>> isJavaVersionAtLeast is easy to understand. What does GTE stand for?
>> 
>>> 
>>> I think I mentioned it a while back but have warnings emitted with 
>>> System.err can be problematic (gets mixed up with application messages to 
>>> stderr). I realize the Xerces code seems to do this in places but we really 
>>> need to see about eliminating these messages or getting consistent logging 
>>> into this code.
>> 
>> I agree, this one is not particularly graceful. There were 88 matches of 
>> System.err in Xalan and 75 in Xerces, although some I believe are used for 
>> debugging. It could take quite some effort.
>> 
>> I mentioned that with a standalone release, we were trying to stay away from 
>> new JDK features. It's probably better to spend time/effort on some upgrades.
>> 
>>> 
>>>> :
>>>> 
>>>> The last scenario to work on is if FSP is set on the Validator instead of 
>>>> SchemaFactory. With that, I'm looking at refactoring the way properties 
>>>> are represented so that they carry state. It would then be cleaner to pass 
>>>> them from SchemaFactory over to Schema and then Validator. It's a bit of 
>>>> work. Fortunately, we only have three of them to deal with.
>>> So are you planning to send another webrev or do it as a separate issue?
>> 
>> Looking at affected code by this change, it doesn't seem to be too bad. I'll 
>> send another webrev.
>> 
>> Joe
>> 
>>> 
>>> -Alan
>> 
> 

Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
[email protected]

Reply via email to