Review Request 113302: Static KEncodingFileDialog in staging/kio

David Faure faure at kde.org
Sat Oct 19 08:58:09 UTC 2013


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113302/#review41957
-----------------------------------------------------------

Ship it!


after a few small fixes


kio/kfile/kencodingfiledialog.h
<http://git.reviewboard.kde.org/r/113302/#comment30606>

    should be kio/kiofilewidgets_export.h, rather



kio/kfile/kencodingfiledialog.h
<http://git.reviewboard.kde.org/r/113302/#comment30605>

    Missing KIOFILEWIDGETS_EXPORT



kio/kfile/kencodingfiledialog.h
<http://git.reviewboard.kde.org/r/113302/#comment30621>

    If you export the class again, no need to export the static methods individually.
    
    Or was it intended to make the class private, even with this new solution? (in that case it should be documented, the ctor should be private, etc.)



kio/kfile/kencodingfiledialog.h
<http://git.reviewboard.kde.org/r/113302/#comment30622>

    "virtual" not needed, with Q_DECL_OVERRIDE



staging/kio/src/filewidgets/kencodingfiledialog.cpp
<http://git.reviewboard.kde.org/r/113302/#comment30623>

    spaces around << (several places in this file)



staging/kio/src/filewidgets/kencodingfiledialog.cpp
<http://git.reviewboard.kde.org/r/113302/#comment30624>

    spaces around =


- David Faure


On Oct. 17, 2013, 2:04 p.m., Teo Mrnjavac wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113302/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2013, 2:04 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Kevin Ottens.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This makes KEncodingFileDialog static, moves it to staging/kio and moves some stuff from KFileDialog to KFileWidget to reduce duplication.
> 
> 
> Diffs
> -----
> 
>   kio/CMakeLists.txt ac80e393c877280dd8033ec13e8e772afea6d2f9 
>   kio/kfile/kencodingfiledialog.h abb939abeb000126005f1a1a9c6fd50b8bd322bd 
>   kio/kfile/kencodingfiledialog.cpp d55d908473aae2859838d88fd776cc65cecf3317 
>   kio/kfile/kfiledialog.cpp 32eb98a4490a31c5ed51150ca3cb7af5375dc67e 
>   staging/kio/src/filewidgets/CMakeLists.txt e8d8c2edf11ad6352e13eb6e7436f828a4a55e3a 
>   staging/kio/src/filewidgets/kencodingfiledialog.cpp PRE-CREATION 
>   staging/kio/src/filewidgets/kfilewidget.h f7b162ab39b997274b21f9eff0c6374545e0a9ad 
>   staging/kio/src/filewidgets/kfilewidget.cpp 824ac563f3f8c463426f0a45e792f107ac402a40 
> 
> Diff: http://git.reviewboard.kde.org/r/113302/diff/
> 
> 
> Testing
> -------
> 
> Seems to work fine in a test app.
> 
> 
> Thanks,
> 
> Teo Mrnjavac
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131019/70a4a805/attachment.html>


More information about the Kde-frameworks-devel mailing list