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

[Martin Basti]

On 30/07/15 06:22, Fraser Tweedale wrote:
> 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
>

ACK

-- 
Martin Basti