<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Vincent / Jason, <br>
Please review the updated webrev based on Jason's comments. <br>
webrev: <font class="" face="Menlo"><a
href="http://cr.openjdk.java.net/%7Etyan/raghu/8049039/webrev02/"
class="">http://cr.openjdk.java.net/~tyan/raghu/8049039/webrev02/</a><br>
<br>
Thanks,<br>
Raghu Nair<br>
</font>
<div class="moz-cite-prefix">On 8/27/2014 9:43 AM, raghu k.nair
wrote:<br>
</div>
<blockquote cite="mid:53FD5B06.2020607@oracle.com" type="cite">Hi
Jason,
<br>
Thanks for your review comments.
<br>
On 8/26/2014 11:37 PM, Jason Uh wrote:
<br>
<blockquote type="cite">Hi Raghu,
<br>
<br>
I'm not an official Reviewer, but I have some comments, mostly
about X509Test.java.
<br>
<br>
1. I'm not sure if this matters, but the formatting of your
copyright header is a little strange. The text appears to be
correct, but the line breaks occur at non-standard places. If
you look at any other file, the first couple lines would look
like this:
<br>
<br>
/*
<br>
* Copyright (c) 2014, Oracle and/or its affiliates. All rights
reserved.
<br>
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
<br>
<br>
Again, not sure how important this is, but it applies to every
file in the webrev. I'd suggest changing them.
<br>
<br>
</blockquote>
This happened due to default formatting by Netbeans . I will
correct it.
<br>
<blockquote type="cite">2. checkMethod():
<br>
Doesn't appear to be used anywhere. Do you want to keep this? If
you do, could you add a comment to explain the parameters?
<br>
<br>
Also, in general for the rest of the methods, there are a lot of
parameters noted with the @param tag. If you want to leave those
javadoc style comments in, could you please describe the params?
<br>
<br>
</blockquote>
X509Test is used by many other tests ( appart from these tests as
part of the webrev). I will add comment and description for the
params.
<br>
<br>
<blockquote type="cite">3. equals() and assertByteArray() both
seem to be "assertEquals()"-style methods, but they are named a
bit differently. could you make the naming consistent? For
example, maybe assertEquals / assertByteArrayEquals.
<br>
<br>
</blockquote>
Yes it make sense. I will make the necessary changes .
<br>
<br>
4. getElementesTest() should probably read getElementsTest(). You
use this method in IssuerAlternativeNameExtensionTest.java, by the
way.
<br>
<br>
typo needs to be fixed.
<br>
<br>
Thanks,
<br>
Raghu
<br>
<blockquote type="cite">Otherwise, I think the tests look good to
me, but again, I'm not a Reviewer, so you still need an official
review.
<br>
<br>
Thanks,
<br>
Jason
<br>
<br>
<br>
<br>
On 08/25/2014 03:59 AM, raghu k.nair wrote:
<br>
<blockquote type="cite">Hi Vincent / Jason,
<br>
Could you please help in reviewing the following test.
<br>
<br>
Thanks,
<br>
Raghu Nair
<br>
<br>
On 8/12/2014 10:28 AM, raghu k.nair wrote:
<br>
<blockquote type="cite">Hello,
<br>
Please review the tests for sun.security.x509 classes.
<br>
These cover tests for GeneralName, GeneralNames,
GeneralSubtree,
<br>
GeneralSubtrees, IPAddressName and
IssuerAlternativeNameExtension.
<br>
<br>
webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~rhalade/8049039/webrev.00/">http://cr.openjdk.java.net/~rhalade/8049039/webrev.00/</a>
<br>
<br>
Bug:
<br>
<a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8049039">https://bugs.openjdk.java.net/browse/JDK-8049039</a>
<br>
<br>
Thanks,
<br>
Raghu Nair
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>