Code review comment for lp:~zeitgeist/zeitgeist/autoload-extensions

Revision history for this message
Markus Korn (thekorn) wrote :

First of all: I don't zhave enough autotools knowlege to understand what the non-python part of this patch is all about, but some test installations show that it seems to work as expected.

This is why I only had a closer look at the python part, it is looking perfect to me, BUT I have a few minor comments:

dict.get(<key>) always returns None if <key> is not found in dict, this is why os.environ("ENVVAR_NAME", None) should be simplified to os.environ("ENVVAR_NAME")

And I've a second, not too serious comment: You are doing to much C stuff! - this explains to me the ugly extra spaces you are adding to the code ;)
Some examples:
* extensions = _scan_extensions () or _zg = __import__ ("_zeitgeist.engine.extensions." + mod)
* log.debug("Searching for extensions in: %s" % config.extensiondir) or modules = filter(lambda m : m.endswith(".py"), os.listdir(config.extensiondir))

Other that that, big Approval.

thnks for your work,

markus

review: Approve (python parts)

« Back to merge proposal