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

[Tomas Babej]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130620/21297152/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0067-2-use-posix-option-that-forces-trusted-range-type.patch
Type: text/x-patch
Size: 5756 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130620/21297152/attachment.bin>