[Bug 69479] Re: SRU: katapult

[Colin Watson]

The X-Katapult-ID change and the KURL setPath/addPath/cleanPath changes
look fine for edgy-proposed.

Some qDebug statements are removed. Is this a good idea? It's not a
minimal change, at any rate; if it's not necessary to remove them then
please leave them be.

The sqlResult[2]=="-1" test was inverted, even though the body of each
branch of that if seems to be logically the same. Was this intentional?

What is the purpose of this change?

-                               QString sqlQuery("SELECT a.name, t.title, t.deviceid, d.lastmountpoint, t.url, i.path, album.name FROM tags t, artist a, album LEFT JOIN statistics s ON t.url = s.url AND t.deviceid = s.deviceid LEFT JOIN images i ON (a.name = i.artist AND album.name = i.album) LEFT JOIN devices d ON t.deviceid = d.id WHERE t.album = album.id AND t.artist = a.id"); // AND
+                               QString sqlQuery("SELECT a.name, t.title, t.deviceid, d.lastmountpoint, t.url, i.path, album.name FROM tags t LEFT JOIN statistics s ON t.url = s.url AND t.deviceid = s.deviceid LEFT JOIN artist a ON t.artist = a.id LEFT JOIN album ON t.album = album.id LEFT JOIN images i ON ( a.name = i.artist AND album.name = i.album) LEFT JOIN devices d ON t.deviceid = d.id WHERE 1");

It appears to be moving stuff from a WHERE into a LEFT JOIN. Is this
necessary? Please explain. My SQL knowledge isn't adequate to approve
this without an explanation.

The changelog is still not satisfactory for edgy-proposed. As Matt said,
"Update of kubuntu_06_amarok_14.diff to working version" is not good
enough. Please explain *what changes were made to that file*! For
example, assuming that only the KURL changes are necessary, something
like "Canonicalise dynamic collection URLs before passing them to
amarok" would be more helpful (although please don't just copy my text
as I'm sure you know the content of the change better than I do, and so
should be able to describe it better). The reason why you need to do
this is that this is the kind of level on which we need to be thinking
when approving changes, and it's the sort of explanation that we ought
to give to reasonably technically-minded users when explaining to them
why we chose to issue a stable release update (particularly in the event
that the update breaks).

-- 
SRU: katapult
https://launchpad.net/bugs/69479