Aaron Seigo wrote: > m_filePlaces = new KFilePlacesModel() gets leaked; Didn't know that, I think I have some other code to check now!
> the object name doesn't actually need to be translated anymore, so this: Done > it would be good to show an error, perhaps via KMessageBox, when setup > fails in setupComplete; silent failures suck for the user. I managed to get it to fail (by breaking my fstab), and as guessed my code does nothing. I then tried doing the same thing in Device Manager, and that just opens Dolphin in the wrong place. Would it make more sense for the error message to go in the code KFilePlacesModel calls in requestSetup? That way we only need one error message not one per app using it. > if you'd like to add this to kdeplasma-addons you may; if you (or someone > else suggests) ... personally i think this belongs in kdebase given how key the places model > has become (file dialog, dolphin, kickoff, gwenview, etc) I've added it to kdeplasma-addons. Then it can be moved if people think otherwise. Sebas wrote: > http://techbase.kde.org/Policies/Kdelibs_Coding_Style I tried. I even did brackets the wrong way { like this } :-P I've now run it through the astyle script that's at the bottom of the page you linked to. Seems to match. > - what's kdDebug? (I've never heard of this, maybe something useful to > know?) kdDebug() is a way of printout to the screen during execution, if you debug code with hundreds of printf statements instead of gdb kdDebug() is the way to go. Amarok use an enhanced version which automatically adds which function you enter. I think it only goes in if debug mode is enabled at compile, but I may be wrong. Removed them all now anyway. > - Why are you linking QTSCRIPT? No idea. I think I stole the CMake file from somewhere else. Was linking to KIO randomly too. Fixed. > - Please add a comment line to the .desktop file Done. I went with "Open Devices and Folder Bookmarks" > - Maybe we need a proper icon and not re-use the bookmarks one? I was torn between the Home folder icon (as it's normally at the top of most parts using the kfileplacesmode, and using the computer icon as used by Device Notifier. I went with the home folder, but it can be changed. _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel