Ade, Thank you for the review. Per our discussion. The agreed changes can now be found at: https://bugzilla.redhat.com/attachment.cgi?id=699762&action=diff&context=patch&collapsed=&headers=1&format=raw In addition, I have added changes to the admin cert profile to accept ECC: https://bugzilla.redhat.com/attachment.cgi?id=699763&action=diff&context=patch&collapsed=&headers=1&format=raw Also, I have now put the ECC setup instruction at this location: http://pki.fedoraproject.org/wiki/ECC_Setup_Instructions So the silentEC_readme.txt now only contains link to it: https://bugzilla.redhat.com/attachment.cgi?id=699764 thanks! Christina On 02/19/2013 12:04 PM, Ade Lee wrote:
Comments:
In general, the code changes look fine. Just a couple of nitpicks ..
1. The variable save_p12 should be a boolean rather than a string.
2. In ComCrypto.java, you define setTokenName(). What about getTokenName()?
3. In ComCrypto.java,on line 448, you cast token to (PK11Token) and on 450, there is no cast. Why the discrepancy? Is the cast needed?
4. The exception handling in ComCrypto.java for lines 448/450 is pretty wonky. It seems like token could return null. Could we improve the exception handling in this case?
Ade
On Tue, 2The 013-02-19 at 11:26 -0800, Christina Fu wrote:
This is a request for code review for the following feature bug: Bug 810967 - [RFE] ECC support for pkisilent
the code changes can be found here: https://bugzilla.redhat.com/attachment.cgi?id=699587&action=diff&context=patch&collapsed=&headers=1&format=raw
The 5 new templates and one readme instruction file can be found the the bug attachment.
thanks! Christina
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel