Review Request 122911: dolphin: SpaceInfoToolsMenu: introduce KMoreTools

[Emmanuel Pescosta]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122911/#review79102
-----------------------------------------------------------


Thanks for your work! :)


src/CMakeLists.txt (line 174)
<https://git.reviewboard.kde.org/r/122911/#comment54050>

    I don't like the idea of adding (and installing) desktop files of other applications to Dolphin.



src/statusbar/spaceinfotoolsmenu.h (line 45)
<https://git.reviewboard.kde.org/r/122911/#comment54051>

    Maybe make this class a menu factory?



src/statusbar/spaceinfotoolsmenu.h (line 49)
<https://git.reviewboard.kde.org/r/122911/#comment54052>

    Please use m_* ;)



src/statusbar/spaceinfotoolsmenu.cpp (line 35)
<https://git.reviewboard.kde.org/r/122911/#comment54053>

    Why so specific? I would make it more generic like {"diskusage", "partitions", ...} so that other applications can make use of it too.
    
    Just pass on the purposes and KMoreTools displays the right tools :)



src/statusbar/spaceinfotoolsmenu.cpp (line 50)
<https://git.reviewboard.kde.org/r/122911/#comment54056>

    Why do we need to specify the applications for the current purpose in the client? IMO KMoreTools should provide all suitable tools for the given purpose(s), am I wrong?



src/statusbar/spaceinfotoolsmenu.cpp (line 51)
<https://git.reviewboard.kde.org/r/122911/#comment54055>

    IMO such information should be stored in the application's desktop files.



src/statusbar/spaceinfotoolsmenu.cpp (line 59)
<https://git.reviewboard.kde.org/r/122911/#comment54057>

    IMO such things should also be defined in the application's desktop files. Maintaining such information for all the tools on our own will become really hard, when the number of available tools increases.


- Emmanuel Pescosta


On April 12, 2015, 8:46 p.m., Gregor Mi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122911/
> -----------------------------------------------------------
> 
> (Updated April 12, 2015, 8:46 p.m.)
> 
> 
> Review request for Dolphin and Emmanuel Pescosta.
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> Move from old dolphin repo (kde-baseapps, https://git.reviewboard.kde.org/r/122352/)
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 89a4e431c46e1f26ab4250420e8a63f04f106f02 
>   src/statusbar/kmt-desktopfiles/gparted.desktop PRE-CREATION 
>   src/statusbar/kmt-desktopfiles/gparted.png PRE-CREATION 
>   src/statusbar/kmt-desktopfiles/kdf.desktop PRE-CREATION 
>   src/statusbar/kmt-desktopfiles/org.kde.filelight.desktop PRE-CREATION 
>   src/statusbar/spaceinfotoolsmenu.h 3ca2e184f03b31d10d661c5a931644485ffc46e6 
>   src/statusbar/spaceinfotoolsmenu.cpp 40ca56e19ebec33518bae0c94dcb56318b473290 
>   src/statusbar/statusbarspaceinfo.h 326c419ab3dc709f5bc4ab23f73e727d1106b3a4 
>   src/statusbar/statusbarspaceinfo.cpp 127641e601d1468479ef925f9539ecd31dc12fb7 
> 
> Diff: https://git.reviewboard.kde.org/r/122911/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> less entries by default than before
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/10/77cb7101-49ae-4812-9958-373cc3961a73__less_entries_by_default_than_before.png
> more entries in submenu
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/10/64a2f205-2db1-4f39-a252-74832f8bb196__more_entries_in_submenu.png
> configure menu dialog
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/04/10/e0a46310-7090-4d95-a0c9-198d5783a5eb__configure_menu_dialog.png
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20150417/7a2e4627/attachment.htm>