← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~trb143/openlp/ThemeManager into lp:openlp

 

Review: Needs Fixing
+    file=open(xmlfile)
+    xml =''.join(file.readlines()) # read the file and change list to a 
string
+    file.close()
+    return xml

return open(xmlfile).read()

+            #print  element.tag, element.text
(and others)

My preference is for debugging prints to either be removed completely, or 
logged (possibly at a lower level than DEBUG).

+        words=words.replace(u'\r\n', u'\n')

I don't think this is the right way to deal with line endings. Did this 
content come from a file?

+        verses_text=words.split(u'\n\n')

words.splitlines()

+            verses_text.append(u'\n'.join(v).lstrip()) # remove first \n

Why is the lstrip() needed? The join won't put a \n at the front.

+        #log.debug(u'_render_lines %s', lines)
(and others)

Again, either log or remove is my preference.

+        log.info(u'Items: %s' % self.items)
(and others)

log.info('Items: %s', self.items)

+                    if file.endswith(u'.bmp'):

That will be case sensitive, which is probably not what we want.

 review needs_fixing

-- 
https://code.launchpad.net/~trb143/openlp/ThemeManager/+merge/5233
Your team openlp.org Core is subscribed to branch lp:openlp.



References