----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100141/#review329 -----------------------------------------------------------
Mostly gcc is the best reviewer for this sort of stuff. So the "it compiles? ... ship it!" philosophy works well. anyways after you incorporate the suggestions of mine that you want, just go ahead and merge. src/core-impl/collections/playdarcollection/PlaydarCollection.cpp <http://git.reviewboard.kde.org/r/100141/#comment276> for a pointer that immediately goes out of scope, there really isn't much point in using qweakpointer (or qpointer) src/core-impl/collections/playdarcollection/PlaydarMeta.cpp <http://git.reviewboard.kde.org/r/100141/#comment277> why not just "return (m_collection.data())" src/core-impl/collections/playdarcollection/PlaydarMeta.cpp <http://git.reviewboard.kde.org/r/100141/#comment278> same as above - Ian On 2010-11-08 19:58:42, Andy Coder wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100141/ > ----------------------------------------------------------- > > (Updated 2010-11-08 19:58:42) > > > Review request for Amarok and Mark Kretschmann. > > > Summary > ------- > > Drops QPointer in favor of QWeakPointer in > src/core-impl/collections/playdarcollection, as was done for other code in > [0] and recommended in [1]. > > Nothing works differently, but since it is, in number of changes at least, > non-trivial, (and I've been fairly absent for a while), I figured I ought to > put it up for review. In particular, anyone more familiar with QWeakPointer > checking to see if I did something stupid that might cause hard to find > problems down the road would be appreciated. > > [0] - http://git.reviewboard.kde.org/r/100011/ > [1] - http://git.reviewboard.kde.org/r/100001/ > > > Diffs > ----- > > src/core-impl/collections/playdarcollection/PlaydarCollection.h 31aba11 > src/core-impl/collections/playdarcollection/PlaydarCollection.cpp 845d3fd > src/core-impl/collections/playdarcollection/PlaydarMeta.h 2599b7c > src/core-impl/collections/playdarcollection/PlaydarMeta.cpp 755191a > src/core-impl/collections/playdarcollection/PlaydarQueryMaker.h 277a582 > src/core-impl/collections/playdarcollection/PlaydarQueryMaker.cpp 590c0d8 > src/core-impl/collections/playdarcollection/support/Controller.h 1db079a > src/core-impl/collections/playdarcollection/support/Query.h 01971f7 > src/core-impl/collections/playdarcollection/support/Query.cpp d586047 > > Diff: http://git.reviewboard.kde.org/r/100141/diff > > > Testing > ------- > > Compiled, ran, played around. Worked fine for me. > > > Thanks, > > Andy > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
