Review: Needs Fixing

Hey!

Looks really nice. Here's some pedantic comments:

 - Shouldn't we abort if the database backup can't be created?

 - It'd make more sense for get_schema_version to be private, I don't think 
it's needed anywhere outside.

 - Can you please preserve the comment we had in Python before the "INSERT INTO 
schema_version VALUES" query? Otherwise it's rather confusing.
    # The 'ON CONFLICT REPLACE' on the PK converts INSERT to UPDATE
    # when appriopriate

 - "if (values != null && values[0] != null)" => I don't think 'values` can 
ever be null (what would happen instead is that the callback isn't called), so 
the first half of the check is empty.

 - "*           © 2011 Canonical Ltd." => in the other files we duplicate the 
"Copyright" at the start of each statement. Consistency doesn't hurt :P.
-- 
https://code.launchpad.net/~mhr3/zeitgeist/bb-schema-ver-table/+merge/79928
Your team Zeitgeist Framework Team is subscribed to branch 
lp:~zeitgeist/zeitgeist/bluebird.

_______________________________________________
Mailing list: https://launchpad.net/~zeitgeist
Post to     : zeitgeist@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zeitgeist
More help   : https://help.launchpad.net/ListHelp

Reply via email to