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

On 7/27/2012 3:53 PM, Abhishek Koneru wrote: 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