Review Request 120135: Port KAppTemplate to KF5

[Milian Wolff]

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

Ship it!


Some small nitpicks, otherwise looks good to me - thanks!

I also added some notes on what you could do in the future here.

many thanks for taking up this project.


TODO
<https://git.reviewboard.kde.org/r/120135/#comment46196>

    I'd suggest you put this into a wiki. TODO files in the code base will be outdated quickly and noone will update them. We've had a TODO in kdevelop which was outdated by 10 years or so...



apptemplatesmodel.cpp
<https://git.reviewboard.kde.org/r/120135/#comment46197>

    another item for your TODO: use a QStringLiteral here and everywhere else you use string literals
    
    but do that in a future patch, no need to complicate this patch with it now.



apptemplatesmodel.cpp
<https://git.reviewboard.kde.org/r/120135/#comment46198>

    this repeats the code from above, you should share the code in a static free function



choicepage.cpp
<https://git.reviewboard.kde.org/r/120135/#comment46199>

    when you cleanup the code style, here and elsewhere, you should add a space after foreach, if, else and the like.
    
        foreach (...)



generatepage.cpp
<https://git.reviewboard.kde.org/r/120135/#comment46201>

    future cleanup: use a scoped pointer.



main.cpp
<https://git.reviewboard.kde.org/r/120135/#comment46202>

    thats not required anymore, afaik. just use i18n("...") below - this is now possible since you initialize the QApplication before


- Milian Wolff


On Sept. 11, 2014, 10:39 a.m., Simon Wächter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120135/
> -----------------------------------------------------------
> 
> (Updated Sept. 11, 2014, 10:39 a.m.)
> 
> 
> Review request for KDevelop and Jonathan Riddell.
> 
> 
> Repository: kapptemplate
> 
> 
> Description
> -------
> 
> This is an initial port of the KAppTemplate application to KF5. These things were changed:
> 
> - Updated all CMake files
> - Ported the codebased including the test cases (Everything still depends on KDELibs4Support)
> - Changed the icon names, so they are installed in the right direction
> 
> There are still several smaller issues like a non working help menu, wrong titles (KDE 4), a missing application icon and outdated templates in general. In the next day I will fix these problems. For more information see the TODO file.
> 
> There is also the idea to create a shared library, so KAppTemplate and the app template wizard of KDevelop can share the same codebase. With this step, we could also think about renaming KAppTemplate to KTemplates, so people can use file and application templates - this step would also be quite suitable for KDevelop. 
> 
> Opinions about that ?
> 
> If people accept the patch I will create a KF5 branch.
> 
> 
> Diffs
> -----
> 
>   .gitignore PRE-CREATION 
>   CMakeLists.txt 0c162c4 
>   TODO PRE-CREATION 
>   apptemplateitem.cpp 01bbc5f 
>   apptemplatesmodel.h c54994a 
>   apptemplatesmodel.cpp 5b62bbd 
>   choicepage.cpp aa4bf9c 
>   cmake/modules/KAppTemplateMacro.cmake 1dd61b2 
>   doc/CMakeLists.txt f9ca3cc 
>   generatepage.cpp fde5c75 
>   icons/CMakeLists.txt 69b754f 
>   icons/hi128-app-kapptemplate.png fed1062 
>   icons/hi128-apps-kapptemplate.png PRE-CREATION 
>   icons/hi16-app-kapptemplate.png aba9f96 
>   icons/hi16-apps-kapptemplate.png PRE-CREATION 
>   icons/hi22-app-kapptemplate.png 2205f64 
>   icons/hi22-apps-kapptemplate.png PRE-CREATION 
>   icons/hi32-app-kapptemplate.png 2c87092 
>   icons/hi32-apps-kapptemplate.png PRE-CREATION 
>   icons/hi48-app-kapptemplate.png 5e896b9 
>   icons/hi48-apps-kapptemplate.png PRE-CREATION 
>   icons/hi64-app-kapptemplate.png 4f4062d 
>   icons/hi64-apps-kapptemplate.png PRE-CREATION 
>   icons/hisc-app-kapptemplate.svg e7d38fa 
>   icons/hisc-apps-kapptemplate.svg PRE-CREATION 
>   kapptemplate.cpp a221ac4 
>   main.cpp 068e714 
>   tests/CMakeLists.txt 2a70758 
>   tests/macrosubstitutiontest.cpp 8a35536 
>   tests/namevalidatortest.cpp e75e28a 
> 
> Diff: https://git.reviewboard.kde.org/r/120135/diff/
> 
> 
> Testing
> -------
> 
> Compilation, testing and installing was done under a project neon 5 system. The core functionallity is working fine (For problems see the TODO file)
> 
> 
> Thanks,
> 
> Simon Wächter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140911/33d66bc5/attachment-0001.html>