divmod-dev team mailing list archive

[Tristan Seligmann <mithrandi@xxxxxxxxxxxxx>]

Review: Approve

This all basically looks good, and fixes an annoying long-standing bug, which is great. I just have one very minor thing to mention:

62	+ if getattr(scheduler, 'setServiceParent', None) is not None:
63	+     scheduler.setServiceParent(collection)

It seems a bit awkward to use getattr to (conditionally) retrieve the attribute and then retrieve it again via dot syntax. Perhaps this might be better:

setServiceParent = getattr(scheduler, 'setServiceParent', None)
if setServiceParent is not None:
    setServiceParent(collection)

or even:

getattr(scheduler, 'setServiceParent', lambda ign: None)(collection)

Please go ahead and merge regardless of whether you change this code or not.
-- 
https://code.launchpad.net/~divmod-dev/divmod.org/start-scheduler-when-parent-service-starts-3000-2/+merge/87280
Your team Divmod-dev is subscribed to branch lp:divmod.org.