[Pki-devel] [PATCH] 53-2 Fixes for comments for patch 53
Abhishek Koneru
akoneru at redhat.com
Tue Apr 23 06:19:29 UTC 2013
Please review the patch with fixes for review comments on patch 53.
On Mon, 2013-04-22 at 14:41 -0500, Endi Sukma Dewata wrote:
> On 4/19/2013 1:42 PM, Abhishek Koneru wrote:
> > Please review the patch for trac ticket 217.
> >
> > Added an additional check over the actual result of a revoke/unrevoke
> > operation.
>
> Some comments:
>
> 1. In CertRequestInfo the synchronized keywords don't seem to be
> necessary. CertRequestInfo objects are created to pass REST operation
> results to the client. They don't seem to be used by multiple threads.
>
--Removed the synchronized keywords
> 2. In CertRequestInfoFactory the operationResult uses 'pass/fail' values
> which are duplicated in several places. It might be better to use
> 'success/error' to match RES_SUCCESS and RES_ERROR constants, and these
> values can be put in constant variables in CertRequestInfo:
>
> public static final String REQ_SUCCESS = "success";
> public static final String REQ_ERROR = "error";
>
> This way we can be sure the operationResult will be used consistently.
>
-- Added the constant variables in CertRequestInfo
> 3. In CertRequestInfoFactory the code for operationResult assignment
> seems to be incorrect because it will assign 'fail' to completed
> requests without error. Try cert-request-find, it will show all requests
> for system certs as failed.
>
-- Corrected. Null value for result is given as a SUCCESS
> 4. In CertCLI if the request status is COMPLETE but the operation result
> is 'fail' the status will appear as REJECTED. This could be confusing
> because the request is not actually rejected. I think we should display
> the operation result separately from the status.
>
-- Printing the operation result seperately.
--Abhishek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-akoneru-0053-2-Check-the-actual-result-of-operations-cert-revoke-un.patch
Type: text/x-patch
Size: 11272 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20130423/cd05916a/attachment.bin>
More information about the Pki-devel
mailing list