<html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 06/17/2013 02:34 PM, Ana Krivokapic wrote: </div> <blockquote cite="mid:51BF0247.4040604@redhat.com" type="cite"> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> <div class="moz-cite-prefix">On 06/06/2013 11:10 AM, Tomas Babej wrote: </div> <blockquote cite="mid:51B05207.4010100@redhat.com" type="cite">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. <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/3650">https://fedorahosted.org/freeipa/ticket/3650</a> Tomas <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ Freeipa-devel mailing list <a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a> <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre> </blockquote> 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: <pre wrap="">+ 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: </pre> </blockquote> All fixed. <blockquote cite="mid:51BF0247.4040604@redhat.com" type="cite">4) Some unit tests to cover the behavior of the newly added option would be nice. </blockquote> This is not doable at the moment, we have no unit test framework to test the trust-add command. <blockquote cite="mid:51BF0247.4040604@redhat.com" type="cite"> <pre class="moz-signature" cols="80">-- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.</pre> <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ Freeipa-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a> <a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre> </blockquote> Tomas </body> </html>