Code review comment for lp:~manishsinha/zeitgeist/fix-blacklist-api

Revision history for this message
Manish Sinha (मनीष सिन्हा) (manishsinha) wrote :

> Can someone please summarize for me why we want this string identifier
> 'blacklister' for the client setting a certain blacklist?

Ask Mikkel. IIRC he proposed the idea.

> And why is it not an identifier for the blacklist template itself?

I think, since the blacklist template is an event itself which has an id, then that id can act as an identifier for the template.

> In my understanding it's purely informational, so why do we group the
> templates internally by this string, instead we could just maintain templates
> internally as
> blacklists = set([(<template1>, "client1"), (<template2>, "client1"),
> (<template3>, "client2")])

I still find having a dictionary a better idea. Keys are blacklister and the value contains the blacklist templates set by them.

« Back to merge proposal