[Freeipa-devel] [PATCH 0078] Enable QR code display by default in otptoken-add

[Petr Vobornik]

On 13.11.2014 16:00, Nathaniel McCallum wrote:
> On Thu, 2014-11-13 at 13:17 +0100, Martin Kosek wrote:
>> On 11/13/2014 07:53 AM, Nathaniel McCallum wrote:
>>> On Thu, 2014-11-06 at 15:33 -0500, Nathaniel McCallum wrote:
>>>> This is possible because python-qrcode's output now fits in a standard
>>>> terminal. Also, update ipa-otp-import and otptoken-add-yubikey to
>>>> disable QR code output as it doesn't make sense in these contexts.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4703
>>>
>>> I removed the period from the doc string to maintain consistency with
>>> the rest of the plugin.
>>>
>>> Nathaniel
>>>
>>
>> I am not reviewing, just curious. What is the purpose of --qrcode option with
>> default=True?
>>
>>       takes_options = LDAPCreate.takes_options + (
>> -        Flag('qrcode?', label=_('Display QR code')),
>> +        Flag('qrcode', label=_('Display QR code'), default=True),
>>       )
>>
>> How can user turn it off? Both passing no option and passing --qrcode will get
>> the same result, no? :-)
>>
>> Shouldn't we instead change --qrcode to --no-qrcode and process somehow fill
>> qrcode=0 when it's enabled? (To achieve API compatibility with old clients).
>
> I see three options:
> 1. Don't let the user turn it off from the command line (he can still
> turn it off from the Python API).
>
> 2. Change it to --no-qrcode (API change)
>
> 3. Change the type to Bool (API change)
>
> Like you, I like #2 the best. Attached is an implementation.

I like --no-qrcode as well.

Should we also keep qrcode as 'no_option' to maintain API compatibility 
(but not CLI)?
-- 
Petr Vobornik