If raising an error results in a crash then I don't agree with you. I got a 
crash because this method was used during loading/migrating settings. These can 
be theoretically modified by the users outside of OpenLP. Crashing when this 
happens doesn't sound right!

You might be able to argue that in that case the error should be caught, which 
may be a good idea. I think that returning None which is a valid thing to 
return according to the docs, is way safer than raising an exception.

Also the docs of the function don't mention the thrown TypeError and I highly 
doubt every call catches it or makes sure the parameter is actually a string. 

A compromise might be to check for "not None" first before checking for the 
string type.

-- 
https://code.launchpad.net/~thelinuxguy/openlp/improve-logging/+merge/352939
Your team OpenLP Core is requested to review the proposed merge of 
lp:~thelinuxguy/openlp/improve-logging into lp:openlp.

_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~openlp-core
More help   : https://help.launchpad.net/ListHelp

Reply via email to