Code review comment for lp:~kamstrup/zeitgeist/blacklist

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

Great to have this blacklist framework in a mergable state, however some first comments (I did not run the code yet)

* docstring in load_class(): we are working on zeitgeist and not on zeitgesit ;)
* statement to unload() all extension: list(self.__extensions.iterkeys()) is looking strange, why not self.__extensions.keys() ?
* the popo code made me smile ;) but it is somehow not looking right,
  ** creating a list containing three None objects is looking strange, but ok. I think
  popo = list()
  popo.append(map(unicode, ev[0]))
  [...]
     is looking much nicer
  ** the for loop looks wrong, I think the body must popo[1][i] = map(unicode, subj)

  I think something like this is more readable:

>>> popo = list()
>>> popo.append(map(unicode, ev[0]))
>>> popo.append([map(unicode, subj) for subj in ev[1]])
>>> popo.append(str(ev[2]))

review: Needs Fixing (diff)

« Back to merge proposal