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@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@redhat.com >> https://www.redhat.com/mailman/listinfo/freeipa-devel > > Tomas
ACK -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel