[Freeipa-devel] [PATCH 0067] Add --use-posix option that forces trusted range type

[Ana Krivokapic]

On 06/20/2013 12:56 PM, Tomas Babej wrote:
> On 06/17/2013 02:34 PM, Ana Krivokapic wrote:
>> On 06/06/2013 11:10 AM, Tomas Babej wrote:
>>> Hi,
>>>
>>> Adds --use-posix option to ipa trust-add command. It takes two
>>> allowed values:
>>> 'yes' : the 'ipa-ad-trust-posix' range type is enforced
>>> 'no' : the 'ipa-ad-trust' range type is enforced
>>>
>>> When --use-posix option is not specified, the range type should be
>>> determined by ID range discovery.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3650
>>>
>>> Tomas
>>>
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>
>> The patch works nicely, but I have a few comments:
>>
>> 1) You added a new option to the API, but you forgot to bump the
>> IPA_API_VERSION_MINOR in the VERSION file.
>>
>> 2) Typo in commit message: "shold" instead of "should".
>>
>> 3) This construct:
>>
>> +            if range_type is not None:
>> +                if range_type != old_range_type:
>>
>> can be replaced with a more readable variant which also avoids nested ifs:
>>
>> +            if range_type and range_type != old_range_type:
>>
>
> All fixed.
>
>> 4) Some unit tests to cover the behavior of the newly added option would be nice.
>
> This is not doable at the moment, we have no unit test framework to test the
> trust-add command.
>
>> -- 
>> Regards,
>>
>> Ana Krivokapic
>> Associate Software Engineer
>> FreeIPA team
>> Red Hat Inc.
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>
> Tomas

ACK

-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130620/eb562f50/attachment.htm>