Review for python patch for upgradeprovision
jelmer at samba.org
Fri Aug 13 15:53:25 MDT 2010
On Sat, 2010-08-14 at 01:37 +0400, Matthieu Patou wrote:
> Could you have a look at this patches ?
This review covers the python side of things mostly, I'm not too
familiar with the ins and outs of upgrading. That said..
Nice branch! Some minor (emphasis on minor) nitpicks:
* in 11118e0e89652b9545dbcabf572c583554b7c797 you add a print statement
to upgradehelpers.py - was that intentional or just for debugging? I
don't think helper functions should write to stdout, so they can e.g. be
used by guis or by tests while still being useful to the user.
* you can give a default value to dict.get(), which will be used if the
key was not found. Rather than:
val = e.get("msDs-KeyVersionNumber")
if not val:
val = "0"
You can also use:
val = e.get("msDs-KeyVersionNumber", "0")
* This seems a bit strange:
+ for ent in current:
+ schema_ldif = "%s%s" % (schema_ldif,
+ samdb.write_ldif(ent, ldb.CHANGETYPE_NONE))
More logical seems:
for ent in current:
schema_ldif += samdb.write_ldif(ent, ldb.CHANGETYPE_NONE)
Or, if you're a fan of list comprehensions:
schema_ldif += "".join([samdb.write_ldif(ent, ldb.CHANGETYPE_NONE)
for ent in current])
* The docstring of int64range2str has an unnecessary leading space
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 836 bytes
Desc: This is a digitally signed message part
More information about the samba-technical