Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
On 31/07/15 14:22, Martin Basti wrote: 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 Ticket for this patch created: https://fedorahosted.org/freeipa/ticket/5171 I amended commit message, ticket was added. Pushed to: ipa-4-2: 3332a0a7c83a9e35532327252231b71e07a44f13 master: 896783bae817ef16ca1cb31a0c434fe863287cc3 -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
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. I will do review is nobody is against this patch :) Martin^2 -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
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. I will do review is nobody is against this patch :) Martin^2 Is a ticket somewhere for this? -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
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. -- Martin Basti -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [PATCH] 0028 add --out option to user-show
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 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
