← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~tomasgroth/openlp/mupdf into lp:openlp

 

Review: Needs Fixing

Looks good but these need to be fixed.

* settings screen the grouping has moved from the left side to the middle.
* settings screen "binary" has the "y" half chopped off.
* showing an image live / preview does not work when loaded from the service manager.  Fine from the media manager.  Have not tried to save / reload but guess that is broken.
* blank lines in functions please remove (general)
* questions in comments please removed.
* check_binary why is this outside the class as a standalone function?
* if os.name != u'nt':  swap logic round so is positive and less chance of a miss read.
* 320 would this be better to have in the code a file instead of writing to a file each run?
* if self.pdf_program_path.text() != u'': should be if not self.pdf_program_path.text():  - General point.
* setting do not need "given" in the string.
* img  should be image (old bad code copied!
* no tests. 
* line 53 far too long.
* lines 52-64 do we need all this.  if the images are generated then it would be a case of just loading them not processing them again?





-- 
https://code.launchpad.net/~tomasgroth/openlp/mupdf/+merge/182009
Your team OpenLP Core is subscribed to branch lp:openlp.


References