Code review comment for lp:~seif/zeitgeist/add_cache_for_get_events

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

Omg, not again, please. If you want to use this lru cache implementation, please create a debian package of this module, get it into debian/ubuntu and add this package as zg dependency. but please don't copy their code into ours (btw, I would consider it as rude if someone copies my code without leaving a comment pointing to the source of the code)

Seif, I'm sure we can fix our implmentation, *if* we know what's going wrong. And from what I see you where not able to provide this information. We know there is a KeyError in __delitem__ at some point. And we know that this is reproducable by you, that's all. Can you please give us more information on the state of the cache when it crashes? has the cache reached its max size? has the event with this id ever been in the cache?

Side note: please *never* merge this branch into lp:zeitgeist, it contains too much trash in the revision history, if you think you found the best solution for such cache please prepare a new clean branch and propose this branch for merge.

« Back to merge proposal