Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/dbschema4 into lp:zeitgeist

2011-03-14 Thread Mikkel Kamstrup Erlandsen
Review: Approve
Awesome Siegfried :-)

Code looks good to me and the tests all run. I think we may want to make a 
backup copy in the upgrade script - before we do anything. This is by far the 
most complex upgrade we have.

Does that mean 0.7 is compatible with the amendment you made to the INSERT? In 
that case I think it could make sense with a 0.7 point release when we roll 0.8.
-- 
https://code.launchpad.net/~zeitgeist/zeitgeist/dbschema4/+merge/52265
Your team Zeitgeist Framework Team is subscribed to branch 
lp:~zeitgeist/zeitgeist/storagemonitor2.

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


Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/dbschema4 into lp:zeitgeist

2011-03-12 Thread Siegfried Gevatter
OK, so I've moved the new columns to the end of the table and now Zeitgeist 0.7 
is at least able to read an upgraded database. Unfortunately it looks like we 
won't get around breaking compatibility for inserts, since:
 > OperationalError: table event has 15 columns but 13 values were supplied

I've changed the INSERT statement to list all columns explicitly so if we ever 
need to change the table in the future this won't happen again.
-- 
https://code.launchpad.net/~zeitgeist/zeitgeist/dbschema4/+merge/52265
Your team Zeitgeist Framework Team is subscribed to branch 
lp:~zeitgeist/zeitgeist/storagemonitor2.

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


Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/dbschema4 into lp:zeitgeist

2011-03-07 Thread Siegfried Gevatter
Indeed, thanks for catching that.

There's one additional issue, as discussed on IRC:
19:07  seiflotfy: should ZG 0.7 and 0.8 be compatible?
19:08  RainCT, how so ur asing
19:09  ZG isn't specifying the column names when it creates an event
19:09  so if you update to 0.8 and run 0.7 it will put stuff in the 
wrong columns
19:11  I think a workaround would be adding the new columns to the end 
of the table (which sucks since the table will look ugly :P) and then having a 
triger which looks for inserts into events
19:11  and if subj_id_current is empty sets it to the same value as 
subj_id
19:26  making the db non-backwards compatible would probably be better
19:26  of course do a backup before converting it first
-- 
https://code.launchpad.net/~zeitgeist/zeitgeist/dbschema4/+merge/52265
Your team Zeitgeist Framework Team is subscribed to branch 
lp:~zeitgeist/zeitgeist/storagemonitor2.

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


Re: [Zeitgeist] [Merge] lp:~zeitgeist/zeitgeist/dbschema4 into lp:zeitgeist

2011-03-05 Thread Mikkel Kamstrup Erlandsen
Review: Needs Fixing
I haven't properly reviewed this yet - but this caught my eye:

48  + CONSTRAINT actor_fk FOREIGN KEY(origin)
49  + REFERENCES uri(id) ON DELETE CASCADE,

I think it should be CONSTRAINT origin_fk instead, right?
-- 
https://code.launchpad.net/~zeitgeist/zeitgeist/dbschema4/+merge/52265
Your team Zeitgeist Framework Team is subscribed to branch 
lp:~zeitgeist/zeitgeist/storagemonitor2.

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