openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #24274
Re: [Merge] lp:~alisonken1/openlp/openlp-2.1-projector into lp:openlp
Review: Needs Fixing
Take 2
Class Projector IP = 100 char. How can this be this long.
r = self.get_object_filtered(object_class=Projector, R should be a proper. There are others as well right across the request
Nice debug in Projector class but OpenLPMixin may have made this easier!
Update a projector with no Projector is a fatal error not debug and drop out.
No blank lines inside methods please
No Blank line at the end of db.py.
families and protocol constants should be in CAPS.
networkutils.py should be moved to projector.__init__.py if these are utility functions.
__get_constants __get_Names should not start with __
projectorconstants.py should also be in the __init__
projectormanager.py you do not need debug for each method that is done by openlpmixin.
Should the thread be created by add_projector?
Wizard class methods missing comments.
You do not need to add numbers to the tests just unique test names and comments
Assert messages talk about what should have happened. "This request should be merged"
The IP4 validation could be used in Remote? Can you check and if so place the tests in common.__init__
--
https://code.launchpad.net/~alisonken1/openlp/openlp-2.1-projector/+merge/235050
Your team OpenLP Core is subscribed to branch lp:openlp.
References