openlp-core team mailing list archive

[Raoul Snyman <raoul.snyman@xxxxxxxxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing
Muhahaha! The Style Police has arrived!

(Yes, this is mostly old code, but it needs to be said!)

All strings are single-quoted and unicode:

  self.display.output("Songs processing started")

  should be:

  self.display.output(u'Songs processing started')

Why are you adding columns one at a time?

  conn.execute("""create table authors (id integer primary key ASC AUTOINCREMENT);""")
  conn.commit()
  self.display.sub_output("authors table created")
  conn.execute("""alter table authors add column first_name varchar(128);""")
  conn.commit()
  self.display.sub_output("first_name added")
  conn.execute("""alter table authors add column last_name varchar(128);""")
  conn.commit()

  Shouldn't that rather be:

  conn.execute("""
    create table authors (
      id integer primary key ASC AUTOINCREMENT,
      first_name varchar(128),
      last_name varchar(128),
      display_name varchar(255)
    );""")
  conn.commit()
  self.display.sub_output("authors table created")

Also name variables better:

  # I'm not sure what these two variables are supposed to be
  def run_cmd(self, cmd):
      f_i, f_o = os.popen4(cmd)
-- 
https://code.launchpad.net/~j-corwin/openlp/migration/+merge/6124
Your team openlp.org Core is subscribed to branch lp:openlp.