Re: [Pki-devel] Request for review: ECC support for pkisilent (CA, subCA, DRM, TKS, TPS, OCSP)

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&... In addition, I have added changes to the admin cert profile to accept ECC: https://bugzilla.redhat.com/attachment.cgi?id=699763&action=diff&... 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&... >> >> The 5 new templates and one readme instruction file can be found the >> the bug attachment. >> >> thanks! >> Christina >> >> >> _______________________________________________ >> Pki-devel mailing list >> Pki-devel(a)redhat.com >> https://www.redhat.com/mailman/listinfo/pki-devel