← Back to team overview

openlp-core team mailing list archive

Re: [Merge] lp:~derek-scotney/openlp/songselectfileimport into lp:openlp

 

Review: Needs Fixing syntactic sugar
Lines 297, 298, 300, 301, 306, 309, 314, 317, 336, 339 and 342 can all lose their brackets. Python is intelligent enough to know what needs to be done when :-)

In Python, empty strings equate to False, so if "my_str" is u'', then "if my_str:" will be false. Use this instead of "if len(my_str) > 0:" and "if not my_str" instead of "if len(my_str) == 0:".

Please don't use single-letter or double-letter variables. For example, use "line" over "ln" and "counter" over "i", etc.

In line 296, you have "ln = line.strip()" - it doesn't look like you're using line again, in which case just reassign line: "line = line.strip()".

As Tim said, please put comments on the line above the code you want commented, as opposed to the same line.

Lastly, make sure there's a blank line at the end of the file.
-- 
https://code.launchpad.net/~derek-scotney/openlp/songselectfileimport/+merge/34128
Your team OpenLP Core is subscribed to branch lp:openlp.



References