Review Request 125248: Display name of process which blocks umount / eject

[Kai Uwe Broulik]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150916/4a2b66cd/attachment-0001.html>