----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125248/#review85488 -----------------------------------------------------------
I really like that! The device notifier UI itself is questionable with the popup telling us "the device xyz failed to unmount" rather than just highlighting the device and showing the message in the list directly but this is of course outside the scope of this patch. +1 for adding this into Solid dataengines/devicenotifications/ksolidnotify.h (line 63) <https://git.reviewboard.kde.org/r/125248/#comment59084> We usually don't add "get" to getters dataengines/devicenotifications/ksolidnotify.cpp (line 123) <https://git.reviewboard.kde.org/r/125248/#comment59090> busyApps dataengines/devicenotifications/ksolidnotify.cpp (line 126) <https://git.reviewboard.kde.org/r/125248/#comment59085> Use initializer list {"-m", devicePath} QStringLiteral could be used too I guess dataengines/devicenotifications/ksolidnotify.cpp (line 127) <https://git.reviewboard.kde.org/r/125248/#comment59086> Please don't use waitForFinished as this can easily freeze the entire shell. Make it async. dataengines/devicenotifications/ksolidnotify.cpp (line 129) <https://git.reviewboard.kde.org/r/125248/#comment59087> const & dataengines/devicenotifications/ksolidnotify.cpp (line 131) <https://git.reviewboard.kde.org/r/125248/#comment59088> Q_FOREACH ( Also, we usually use camelCase instead of underscodes dataengines/devicenotifications/ksolidnotify.cpp (line 133) <https://git.reviewboard.kde.org/r/125248/#comment59089> if (!pid) { continue; } ? dataengines/devicenotifications/ksolidnotify.cpp (line 157) <https://git.reviewboard.kde.org/r/125248/#comment59091> Could use plural handling: i18np("Could not … are opened in \"%1\".", "Could not … are opened in the following: %1") - Kai Uwe Broulik On Sept. 16, 2015, 7:23 vorm., Igor Poboiko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125248/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2015, 7:23 vorm.) > > > Review request for Plasma and Solid. > > > Repository: plasma-workspace > > > Description > ------- > > An attempt to implement feature from [bug > 96107](https://bugs.kde.org/show_bug.cgi?id=96107) (see summary). > > On umount/eject error it runs "lsof" executable, gets PIDs of blocking > processes and obtains names of processes via KSysGuard::Process machinery. > Finally it appends obtained information to error message which is shown in > tooltip of Device Notifier applet. > > > Diffs > ----- > > dataengines/devicenotifications/CMakeLists.txt 3f7fd83 > dataengines/devicenotifications/ksolidnotify.h a471d50 > dataengines/devicenotifications/ksolidnotify.cpp 35d49d6 > > Diff: https://git.reviewboard.kde.org/r/125248/diff/ > > > Testing > ------- > > Tested unmount on busy device with several processes blocking it. Did not > test eject, since I have no optical disc drive :( > > > File Attachments > ---------------- > > Applet with error message > > https://git.reviewboard.kde.org/media/uploaded/files/2015/09/15/3b1b64fc-abff-4633-9dca-621388edf086__snapshot11.png > > > Thanks, > > Igor Poboiko > >
_______________________________________________ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel
