Review Request 122305: Port Dolphin from KDialog to QDialog

[Mark Gaiser]

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



dolphin/src/settings/viewpropertiesdialog.h
<https://git.reviewboard.kde.org/r/122305/#comment51981>

    final? Nobody is allowed to inherit from that class?
    
    Well, it's mostly internal dolphin stuff so i don't really see a need to mark something final. Other then that, KDE supports visual studio 2010 and that has only partial support for override and final [1]. They don't really say what partial is.. Anyway, i'd remove it to be on the safe side.
    
    [1] https://msdn.microsoft.com/en-us/library/hh567368.aspx



dolphin/src/settings/viewpropertiesdialog.cpp
<https://git.reviewboard.kde.org/r/122305/#comment51982>

    Auto...
    My preference (feel free to ignore this comment) is to only use auto when it would otherwise be a long line (like iterators) or you really don't know the type it's going to have. In this case you perfectly know what the type will be so i'd not use auto there.
    
    Same for all the other usages of the auto keyword.



dolphin/src/settings/viewpropertiesdialog.cpp
<https://git.reviewboard.kde.org/r/122305/#comment51983>

    Something weird is going on here. I think you can delete the line:
    QWidget* propsBox = this;
    
    and replace the line:
    propsBox = new QGroupBox(i18nc("@title:group", "Properties"), this);
    
    with
    QGroupBox *propsBox = new QGroupBox(i18nc("@title:group", "Properties"), this);
    
    Or will this break other parts?


- Mark Gaiser


On jan 29, 2015, 12:12 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122305/
> -----------------------------------------------------------
> 
> (Updated jan 29, 2015, 12:12 p.m.)
> 
> 
> Review request for Dolphin and Christoph Feck.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Ported Dolphin from KDialog to QDialog and save/restoreDialogSize to KWindowConfig::save/restoreWindowSize with the help of the porting scripts.
> 
> @Christoph
> I have added you to this review request, because you are the layout porting expert from what I haven seen so far ;)
> 
> 
> Diffs
> -----
> 
>   dolphin/src/settings/navigation/navigationsettingspage.cpp 69feed3 
>   dolphin/src/settings/startup/startupsettingspage.cpp 11adb86 
>   dolphin/src/dolphinmainwindow.cpp 398af70 
>   dolphin/src/panels/information/informationpanelcontent.cpp 4e32a79 
>   dolphin/src/panels/information/phononwidget.cpp 215be20 
>   dolphin/src/settings/additionalinfodialog.h 4197d47 
>   dolphin/src/settings/additionalinfodialog.cpp 4d1b69c 
>   dolphin/src/settings/general/configurepreviewplugindialog.h 02a0cde 
>   dolphin/src/settings/general/configurepreviewplugindialog.cpp eb9ad01 
>   dolphin/src/settings/general/confirmationssettingspage.cpp 617d31c 
>   dolphin/src/settings/general/generalsettingspage.cpp f32cb9c 
>   dolphin/src/settings/general/previewssettingspage.cpp 835af9d 
>   dolphin/src/settings/general/statusbarsettingspage.cpp d12a0c1 
>   dolphin/src/settings/kcm/kcmdolphingeneral.cpp 2856336 
>   dolphin/src/settings/kcm/kcmdolphinnavigation.cpp 5a0be61 
>   dolphin/src/settings/kcm/kcmdolphinservices.cpp b00f58a 
>   dolphin/src/settings/kcm/kcmdolphinviewmodes.cpp d9c8a1a 
>   dolphin/src/settings/trash/trashsettingspage.cpp aa4b5a2 
>   dolphin/src/settings/viewmodes/viewsettingspage.cpp 12d4ce0 
>   dolphin/src/settings/viewpropertiesdialog.h 6b0e9ff 
>   dolphin/src/settings/viewpropertiesdialog.cpp 2e503bb 
>   dolphin/src/settings/viewpropsprogressinfo.h 13089b7 
>   dolphin/src/settings/viewpropsprogressinfo.cpp 9ce3d2d 
> 
> Diff: https://git.reviewboard.kde.org/r/122305/diff/
> 
> 
> Testing
> -------
> 
> Compared all the dialogs with Dolphin 14.12 dialogs, the layouts are almost the same.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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