openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Review: Needs Fixing

18  self.main_window.blank_check()

If this is supposed to check if the display is blank, it would be easier to read as main_window.is_display_blank()


426	registry = cls()
427	registry.service_list = {}
429	registry.functions_list = {}
430	registry.running_under_test = True
431	# Allow the tests to remove Registry entries but not the live system
434	if u'openlp.py' in sys.argv[0]:
435	    registry.running_under_test = False
436	return registry

Rather whitelist than blacklist, i.e. test that we're running tests, not that we're running live. Presume running live unless otherwise set up. This prevents lots of potential bugs.


477	+ def register_function(self, event, function):
478	+ """
479	+ Register a function and a handler to be called later

A function AND a handler? A handler usually IS a function. And what about the event? Perhaps rather call it "register_handler"?


494	+ def remove_function(self, event, function):
495	+ """
496	+ Register a function and a handler to be called later

Register or remove?


511	+ def execute(self, event, *args, **kwargs):
512	+ """
513	+ Execute all the handlers registered passing the data to the handler and returning results

If you're dealing with events, generally you trigger them, you don't execute them.
-- 
https://code.launchpad.net/~trb143/openlp/event_receiver_removal/+merge/147525
Your team OpenLP Core is subscribed to branch lp:openlp.