> On 2010-10-01 17:13:46, Aaron Seigo wrote: > > /trunk/KDE/kdelibs/plasma/private/storage.cpp, line 41 > > <http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line41> > > > > instead of an incrementing connectionId, you could also use the value > > of the this pointer? one less static int kicking around.
uhm, i tought using a pointer value was even uglier, but ok. another thing that i tought seeing it. is really necessary having a different connection per job, since the query execution is afaik syncronous? even tought aboout a little singleton that holds a single QSqlDatabase... > On 2010-10-01 17:13:46, Aaron Seigo wrote: > > /trunk/KDE/kdelibs/plasma/private/storage.cpp, lines 46-49 > > <http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line46> > > > > destination() is no longer used; with one db file used for all the > > storage, we end up with all the data in one db with no separation. > > > > it would make sense to me here to create a table for each destination(). > > > > on applet destruction, it could drop its table (if any). > > > > on applet export, it could pull its table over to an exported db. (i > > imagine that Corona would use this in its export method). or, destination could be a field (with key,destination being the primary key) would be ugly, but easier to expire old entries but yeah, i like more the different tbles approach in general also, if this is going to be used by applets, the source field doesn't make sense here... > On 2010-10-01 17:13:46, Aaron Seigo wrote: > > /trunk/KDE/kdelibs/plasma/private/storage.cpp, line 66 > > <http://svn.reviewboard.kde.org/r/5506/diff/1/?file=38787#file38787line66> > > > > ah, sql databases. > > > > this query will result in multiple rows with the same key, and when > > requested back you'll end up with a random row. > > > > to fix this, first delete the old row. or check if the old row exists > > and do an update instead of an insert. and no, there is no "update or > > insert" in sql. that's what sorted procedures are for ;P uhm, i think even worse, since key is primary the inseret should just silently fail, so the old stale entry wins i think i'll just try to delete the line before. - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://svn.reviewboard.kde.org/r/5506/#review7926 ----------------------------------------------------------- On 2010-10-01 15:05:11, Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://svn.reviewboard.kde.org/r/5506/ > ----------------------------------------------------------- > > (Updated 2010-10-01 15:05:11) > > > Review request for Plasma. > > > Summary > ------- > > looking at how slow kconfig as, this makes storage use sqlite (and makes some > methods private, before it's too late) > it can still use some improvements but it's basically working. > two main concerns are: > - is acceptable/safe to link to QtSql and assume the sqlite driver is present? > - i would still see this as a fallback for when an akonadi version is not > present (being in another process should slowdown the gui a bit less, but i > could not want it in some mobile profiles) > > the akonadi version is in the usual status "almost working with developer > disappeared" :p > > > Diffs > ----- > > /trunk/KDE/kdelibs/plasma/CMakeLists.txt 1179394 > /trunk/KDE/kdelibs/plasma/datacontainer.h 1179394 > /trunk/KDE/kdelibs/plasma/datacontainer.cpp 1179394 > /trunk/KDE/kdelibs/plasma/dataengine.cpp 1179394 > /trunk/KDE/kdelibs/plasma/private/datacontainer_p.h 1179394 > /trunk/KDE/kdelibs/plasma/private/storage.cpp 1179394 > /trunk/KDE/kdelibs/plasma/private/storage_p.h 1179394 > > Diff: http://svn.reviewboard.kde.org/r/5506/diff > > > Testing > ------- > > > Thanks, > > Marco > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
