benapetr / wikimedia-bot

IRC bot that is being used on number of wikimedia channels
http://meta.wikimedia.org/wiki/WM-Bot
GNU General Public License v3.0
36 stars 39 forks source link

Add @restrict and @ignore commands #69

Open nikitavbv opened 6 years ago

nikitavbv commented 6 years ago

@restrict restricts bot to join specific channels. @ignore allows to ignore all messages of users specified by hostmasks.

Bug: T144914

benapetr commented 6 years ago

While this might work (I will finish review soonish), I am just wondering, why did you decide to do it this way, by adding some extra variables in Channel class, instead of doing it simply same as you did with users, by creating some extra list with channel names that are forbidden?

That would be easier to implement as it's significantly less coding and probably also way more efficient as you wouldn't need to create whole instance of class Channel for every single banned channel name.

nikitavbv commented 6 years ago

I did it this way because I thought it would be nice to keep this information together with all other information about the specific channel and it would also make it easier to possibly extend this functionality later.

Anyway, I agree with you - creating separate list of banned channels seems to be more efficient. The only thing we need to think about is how to organize stroing all this data optimally. For example, we need to store if bot could be added back to banned channel by admin. So we need to have a second list here containing list of channels which could be unblocked by admin? Or we better have one list, but storing not strings (channel names), but more complex structures? We also need to think about format used in file containing information about blocked channels (maybe xml?)

nikitavbv commented 6 years ago

I think I fixed all issues you pointed at. I also improved unblock permissions logic. Additionally, I moved hostmask ignore command to @ignore.

nikitavbv commented 6 years ago

Could you tell please if there is anything else we need to fix here before this can be merged?

aklapper commented 6 years ago

@benapetr: Any news / opinion / review here?