[Freeipa-devel] [PATCH] 0028 add --out option to user-show

[Fraser Tweedale]

On Thu, Jul 30, 2015 at 10:19:19AM +1000, Fraser Tweedale wrote:
> On Wed, Jul 29, 2015 at 03:48:47PM +0200, Jan Cholasta wrote:
> > Dne 29.7.2015 v 15:46 Martin Basti napsal(a):
> > >On 29/07/15 15:41, Martin Basti wrote:
> > >>On 25/07/15 03:40, Fraser Tweedale wrote:
> > >>>On Fri, Jul 24, 2015 at 05:53:56PM +0200, Tomas Babej wrote:
> > >>>>
> > >>>>On 07/24/2015 05:34 PM, Martin Basti wrote:
> > >>>>>On 24/07/15 16:52, Tomas Babej wrote:
> > >>>>>>On 07/24/2015 03:40 PM, Fraser Tweedale wrote:
> > >>>>>>>The attached patch adds --out option to user-show for saving user's
> > >>>>>>>certificate(s) to file.
> > >>>>>>>
> > >>>>>>>Thanks,
> > >>>>>>>Fraser
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>I hate to nitpick here, but is "out" really a descriptive option name
> > >>>>>>here? I'd prefer something more explicit, like '--save-cert-to', or
> > >>>>>>maybe even have this operation implemented as a separate command
> > >>>>>>altogether.
> > >>>>>>
> > >>>>>>Tomas
> > >>>>>>
> > >>>>>This keyword was already used with several commands. For consistency
> > >>>>>might be better to have it the same.
> > >>>>>
> > >>>>True. I see this options is being used in the following commands:
> > >>>>
> > >>>>  - cert-show
> > >>>>  - vault-retrieve
> > >>>>  - host-show
> > >>>>  - service-show
> > >>>>  - user-show (proposed)
> > >>>>
> > >>>>While the first two seem to be an acceptable fit for an option called
> > >>>>--out, as they mainly deal with cert/secret, using the '--out' for the
> > >>>>latter three is a poor decision imho.
> > >>>>
> > >>>>I agree the consistency is important, I'm just not happy to see this
> > >>>>spread further.
> > >>>>
> > >>>>Tomas
> > >>>Perhaps we should go with something like `--certout' instead, and
> > >>>support `--certout' in addition to `--out' in host-show and
> > >>>service-show, esentially deprecating `--out' for those commands.
> > >>>
> > >>>Cheers,
> > >>>Fraser
> > >>Good idea, but we should do this for all commands, at the same time.
> > >>IMO this is not for 4.2, you may file a ticket to deprecate --out
> > >>option and replace it by --certout or something.
> > 
> > The "in" option is named --certificate, so it should be --certificate-out.
> > 
> > >>
> > >>I will do review is nobody is against this patch :)
> > >>Martin^2
> > 
> > LGTM
> > 
> > >>
> > >
> > >Is a ticket somewhere for this?
> > >
> No ticket; I just wanted it so I wrote the patch :)
> 
> I'll file the ticket for future change to `--certificate-out'
> though.
> 
Ticket: https://fedorahosted.org/freeipa/ticket/5166