Code review comment for lp:~tomasgroth/openlp/mupdf

Revision history for this message
Tim Bentley (trb143) wrote :

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?

review: Needs Fixing

« Back to merge proposal