Refactoring Notification preferences #1655
Conversation
|
@kenmaca nice rename there of the function, makes more sense (although see my OO related comment) You got a test failure :-) in |
|
|
||
| return None | ||
| # Cascades to the first setting (ie, closest to the individual watch settings) that's defined | ||
| return next( |
There was a problem hiding this comment.
return next( hmm I'll have to rtfm about next
|
The other thing that would be amazing to add is a test case that covers atleast part of the notification preferences chain, at the moment the tests only cover doing it exactly with globals, or exactly per-watch etc, never a mixed case... if you wanted to add some test here I would be very grateful |
|
Another reason why tests are amazing, they caught my mistake. When there aren't any settings resolved by any layer, It's been awhile since I've written pytests, so I'll find some time this week to relearn it and update the PR. Thanks! |
Follow up to #1653 and comments around refactoring
_check_cascading_varsto liftwatch.get('notification_muted')out intosend_content_changed_notificationinstead.I renamed the function and put some time into cleaning up the function itself to be more concise, if you don't mind. It's been awhile since I've written anything in Python!