On 09/26/2013 03:16 PM, Petr Viktorin wrote:
On 09/26/2013 02:58 PM, Martin Kosek wrote:
On 09/26/2013 02:45 PM, Jan Cholasta wrote:
On 26.9.2013 14:38, Martin Kosek wrote:
On 09/26/2013 02:28 PM, Tomas Babej wrote:
On 09/26/2013 12:20 PM, Jan Cholasta wrote:
...
I just found --no-nisdomain more descriptive and explicit. If
there is a
consensus, I can remove it.
I am not aware of any precedent that would warrant --nisdomain="".
We sort of have precedent in `ipa` in multivalued options, leaving
those empty deletes the values.
I have seen concerns about the number of ipa-client-install options
in the past
(not by me).
IMHO, we are currently OK on this front. Having options categorized in
sections, as we already do, helps.
IMO --no-nisdomain is more consistent with rest of the options.
I don't see any other --<option>=<value> and --no-<option> option
pair in
ipa-client-install, so what consistency are you talking about?
I was referring to --no-ssh, --no-ntp and similar. But it is true
that these
rather disable entire features than delete a value. I do not punt on
this,
--nidomain="" may be OK as well.
IMO empty option values are awkward; --no-nisdomain is more
user-friendly, and can be explained more clearly, even though it needs
an additional option.
OK, we let this rot on the list for a while.
I retest the patch and it still applies and works with the current master.
I think we should keep both options, no-nisdomain is more descriptive
and an explicit option is more necessary here since we are setting
nisdomain by default. Hence I would avoid having to use --nisdomain=""
to disable setting the nisdomain, since it is rather implicit (even if
we commented on it in the option description).
Option-nitpicking aside, I think this patch is ready for a proper
functional review.
--
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
