[Pki-devel] [PATCH] 26-3 Fix for review comment given for [PATCH] 26-2

[Endi Sukma Dewata]

On 7/27/2012 3:53 PM, Abhishek Koneru wrote:
> Please review the fix for the comment given for patch 26-2

ACK. Pushed to master.

Some possible improvements:

9. The "output" option can be marked as required like this:

   option.setRequired(true);

This way if the option is missing it will fail during parsing, so it's 
not necessary to write a code to check it.

10. The marshalling/unmarshalling code for AgentEnrollmentRequestData 
can be refactored into reusable methods. For example:

   public class AgentEnrollmentRequestData {
     public static AgentEnrollmentRequestData valueOf(Reader reader) { }
     public String toString() { }
   }

11. The use of option/argument to pass the filename is not really 
consistent in the submit/review/approve commands. I don't have a strong 
opinion on this but here are some possibilities. With arguments the 
commands will be shorter:

   pki cert-request-submit <filename>
   pki cert-request-review <request id> <filename>
   pki cert-request-approve <filename>

With options we could have more flexibility:

   pki cert-request-submit --input <filename>
   pki cert-request-review <request id> --ouput <filename>
   pki cert-request-approve --input <filename>

   cat <filename> | pki cert-request-submit
   pki cert-request-review <request id> --json <filename>
   pki cert-request-approve <request id> --nonce <nonce>

We should consider other commands too (reject, cancel, update, etc.).

-- 
Endi S. Dewata