openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

Review: Approve approve
Yes, I know it's not your code, but I'm just saying this for the record...


Use the os.path module functions, they tend to take care of little things:

  conn = sqlite3.connect(self.data_path + os.sep + database)

  should rather be:

  conn = sqlite3.connect(os.path.join(self.data_path, database))


Indices really should have functional names... "author1" is not a good name:

  conn.execute("""create index if not exists author1 on authors (display_name ASC,id ASC);""")

  better:

  conn.execute("""create index if not exists author_display_name on authors (display_name ASC,id ASC);""")


I'm a little wary of all these "replace"s - there's probably a better way...

  replace(replace(replace(replace(replace(replace(replace(replace(


-- 
https://code.launchpad.net/~j-corwin/openlp/migration/+merge/6131
Your team openlp.org Core is subscribed to branch lp:openlp.