openlp-core team mailing list archive

[Andreas Preikschat <andreaspreikschat@xxxxxxxxxx>]

Review: Needs Fixing

There are some syntactic things I'd like to be changed:

148	+ s = QtCore.QSettings()
149	+ keys = s.allKeys()
150	+ for k in keys:
151	+     s.setValue(k, None)

Why not?
148	+ settings = QtCore.QSettings()
149	+ keys = settings.allKeys()
150	+ for key in keys:
151	+     settings.setValue(key, None)

Makes everything much more readable. Consider that over the time more and more code might be added.


226	+ # Xml conforms to schema.
227	+     return True
228	+ else:
229	+ # Xml has invalid syntax.
230	+     return False

Does the same:
return retcode == 0

There are a few blank lines (or two blank lines). E. g.
1200	+
1295	+

We always have only one blank line in between things (methods, functions, ...)  apart from classes which are separated by two blank lines.

In 1542/1543 is a line missing.
Finally, I think that a few docstrings are missing.
-- 
https://code.launchpad.net/~mzibricky/openlp/openlyrics_tests/+merge/77060
Your team OpenLP Core is subscribed to branch lp:openlp.