Code review comment for lp:~divmod-dev/divmod.org/start-scheduler-when-parent-service-starts-3000-2

Revision history for this message
Tristan Seligmann (mithrandi) wrote :

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.

review: Approve

« Back to merge proposal