← Back to team overview

openlp-core team mailing list archive

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