-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Allow updating user_ids/usernames of a running filters.user #1757
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Maybe a comment on the change to sets is in order: I did this because we need only user id/name only once and that way removing a user from the filter is easier. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comments on the code
Two more things:
|
good idea. go for it.
so what i'm saying, sets are good and for large numbers will provide better performance. in small numbers it wouldn't matter if it's a list or set. |
on a second thought, having a general solution might be over engineering. |
Co-authored-by: Noam Meltzer <tsnoam@gmail.com>
Okay. But then I'll wait until we're happy with |
pylint complained on some extra stuff, so cleaned them as well
LGTM |
Fails unrelated. Merging. |
filters.user.{user_ids, usernames}
editable by:user_ids
andusernames
propertiesIf I got that right, this closes #1562