[Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

[Martin Basti]

...
1)
+        if options.unattended:
+            for ip in ip_addresses:
+                if search_reverse_zones and find_reverse_zone(str(ip)):
+                    # reverse zone is already in LDAP
+                    continue
+                for rz in ret_reverse_zones:
+                    if verify_reverse_zone(rz, ip):
+                        # reverse zone was entered by user
+                        break
+                else:
+                    rz = get_reverse_zone_default(str(ip))
+                    ret_reverse_zones.append(rz)
+        elif options.reverse_zones or create_reverse():
+            for ip in ip_addresses:
+                if search_reverse_zones and find_reverse_zone(str(ip)):
+                    # reverse zone is already in LDAP
+                    continue
+                for rz in ret_reverse_zones:
+                    if verify_reverse_zone(rz, ip):
+                        # reverse zone was entered by user
+                        break
+                else:
+                    rz = get_reverse_zone_default(str(ip))
+                    rz = read_reverse_zone(rz, str(ip))
+                    ret_reverse_zones.append(rz)
+        else:
+            options.no_reverse = True
+            ret_reverse_zones = []

You can make it shorter without duplications:

# this ifs can be in one line
if not options.unatended:
     if not options.reverse_zones
         if not create_reverse():
             options.no_reverse=True
             return []

for ip in ip_addresses:
     if search_reverse_zones and find_reverse_zone(str(ip)):
         # reverse zone is already in LDAP
         continue
     for rz in ret_reverse_zones:
         if verify_reverse_zone(rz, ip):
             # reverse zone was entered by user
             break
         else:
             rz = get_reverse_zone_default(str(ip))
             if not options.unattended:
                 rz = read_reverse_zone(rz, str(ip))
             ret_reverse_zones.append(rz)


2)
Typo?     There is no IP address matching reverze_zone %s."
---------------------------------------------^^


3)
Would be nice to ask user to create new zones only if new zones are 
required. (If all required zones exist in LDAP, you ask user anyway)

4)
Ask framework gurus, if installutils module is better place for function 
above



-- 
Martin Basti