On 11/21/2011 09:49 AM, Ade Lee wrote:
On Sun, 2011-11-20 at 20:57 -0500, Adam Young wrote:
On 11/16/2011 10:33 AM, Adam Young wrote:
This is a review of this patch and the subsequent one removing unnecessary blocks.
CMCAuth.java: Can you explain why this code removal is correct? At this point in the code, cert can only be null. The only code
On 11/15/2011 01:11 AM, Ade Lee wrote: path that assigns a value to cert has a return after it, and cannot reach this point. Thus, the code executed when cert != null is unreachable
CAAdminServlet.java : code should be commented out, rather than removed. Disagree. If this code has never been run, it is unnecessary. Lets not put dead code into the source tree. HashEnrollServlet.java : remove the outer conditional as well. Done DBSubsystem.java: some important comments are removed, they should not be removed. Done FileAsString.java - does the proposed code removal introduce a resource leak? No. FileReader can throw a file not found exception. But BufferedReader only throws an IllegalArgumentException, which wouldn't be caught by that catch block anyway.
KeyRecoveryAuthority.java: please explain why the proposed code removal is correct. It certainly looks wrong. I agree that the change looks wrong. I put it back in, and Eclipse did not tag it as dead code. Ade
On Wed, 2011-11-09 at 19:50 -0500, Adam Young wrote:
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel Is that an ACK?
_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel ACK
Commited to trunk. To to the PKISIlent restructuring, I had to move the ArgParser code by hand, but the change matches what was done in the patch.