Hi Max,

Please help to check again: http://cr.openjdk.java.net/~rhalade/8048619/webrev.04/

Also please help to push it if ok:
Full comments:
8048619: Implement tests for converting PKCS12 keystores
Reviewed-by: weijun
Contributed-by: Zaiyao Liu <zaiyao....@oracle.com>

Thanks and Regards.

Kevin


于 2014/12/3 15:37, Wang Weijun 写道:
Looks fine.

One nit, line 231, remove "Certs don't match ".

Thanks
Max

On Dec 3, 2014, at 10:26, zaiyao liu <zaiyao....@oracle.com> wrote:

Hi Max,

Please help to review the update:
http://cr.openjdk.java.net/~rhalade/8048619/webrev.02/

Thanks
Kevin
于 2014/9/23 15:08, zaiyao liu 写道:
Hi Max,

Please help to check this 
update:http://sqeweb.us.oracle.com/net/sqenfs-1/export1/comp/jsn/users/kevin1/webrev/8048619/webrev/

Thanks

Kevin
于 2014/9/22 17:10, Wang Weijun 写道:
On Sep 22, 2014, at 16:11, zaiyao liu <zaiyao....@oracle.com> wrote:

Hi Max,

Thanks for review. please review the update: 
http://sqeweb.us.oracle.com/net/sqenfs-1/export1/comp/jsn/users/kevin1/webrev/8048619/webrev/,
 I will ask Rajan to create open webrev link after you passed.
  194         if (a.size() != b.size()) {

How about "if (a.size() != 1 || b.size() != 1)"?

  239-250

This can be as simple as Arrays.equals(certsA, certsB).

I would still like a single compareKeyStore() no matter what the size of the 
keystore is.

--Max

Thanks

Kevin
于 2014/9/19 15:34, Wang Weijun 写道:
In compareCerts(), you should not compare Certificate.toString(), its equals() 
method is more reliable. There is no need to define compareKeys() and 
compareCerts(). Instead, you should try not to repeat lines 223-231 on 252-260.

The two calls on lines 204 and 205 have keystore names exchanged but keypass 
order is the same. Also, with two calls it means the comparisons of keys and 
certs are duplicated. The else block on line 206 looks strange. Do you mean the 
size is always 1 here? If so, check it. Otherwise, there is no guarantee the 
aliases appear in the same order.

I would be glad to see a generalized comparison method no matter what the 
keystore size is.

Minor issues:

It will be clear if you divide the constant strings at the beginning into 2 
parts, one for provider names, and one for algorithms.

Line 143 and 145, there should be spaces around the testCase name.

--Max

On Sep 19, 2014, at 11:04, zaiyao liu <zaiyao....@oracle.com> wrote:

Hi Max,

Can you help to review it?

Thanks

Kevin
于 2014/9/1 13:25, Wang Weijun 写道:
On vacation now. Can you look for someone else? I will be back in Sep 17 if you 
are not in a hurry.

--Max

On Sep 1, 2014, at 9:37, zaiyao liu <zaiyao....@oracle.com> wrote:

Hi Max,

Please review the code change,the purpose of this fix is implement tests that 
convert PKCS12 keystores to other formats.

Webrev: http://cr.openjdk.java.net/~tyan/kevin/JDK-8048619/webrev01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8048619

Thanks

Kevin

Reply via email to