[Pki-devel] [PATCH] 26-2 Fixes for review comments on[PATCH] 26 CLI - Cert Request Review and Cert Request Approve Implementation

[Abhishek Koneru]

Please find attached the fixes for comments given for Patch26.

--Abhishek Koneru

On Thu, 2012-07-26 at 12:11 -0500, Endi Sukma Dewata wrote:
> On 7/24/2012 4:05 PM, Abhishek Koneru wrote:
> > Please review the patch with implementations for review and approval of
> > a cert request using CLI.
> 
> I have not been able to run it successfully since I'm still working on 
> auth, but I have some comments below. I'll test this once I have a 
> working environment.
> 
> 1. The CertRequestApproveCLI.printHelp() generates a message like this:
> 
>      usage: cert -U <uri> -d <Certificate database directory>
>      -w <password> -n <cert> request-approve <request id>
> 
> The command name is split into "cert ... request-approve". It's supposed 
> to show "cert-request-approve". Also it's not necessary to show the -U, 
> -d, -w, -n options because they are part of the MainCLI's options. You 
> can view those options by executing "pki" without any parameters.
> 
> 2. Same as #1 for CertRequestReviewCLI.printHelp().
> 
> 3. In CAEnrollProfile.java there are some "Check Point <n>" debug 
> messages. I'd say we should either replace them with messages that 
> explain what the code is doing or just remove them.
> 
> 4. In CertRequestResource.java the MediaType.TEXT_XML is actually not 
> needed. We should use MediaType.APPLICATION_XML for XML content type.
> 
> 5. The CertRequestResourceService.assignRequest() returns an empty 
> CertRequestInfos. It doesn't seem to be necessary, but I haven't tested it.
> 
> 6. In CertRequestResourceService.java:163 the original exception should 
> be attached to the new exception as follows:
> 
>      throw new CMSException("Problem approving request in CertRequestRes
> urce.assignRequest!", e);
> 
> 7. In CertRequestResourceService.java:165 the debug message 
> "RequestNotFound" below is not necessary because the toString() will 
> include the exception name in the output. It should be just:
> 
>      CMS.debug(e);
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-akoneru-0026-2-Cert-CLI-cert-request-review-and-cert-request-approv.patch
Type: text/x-patch
Size: 14179 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20120726/1fa20764/attachment.bin>