Review Request 120571: Port KAppTemplate away from KDELibs4Support

Simon Wächter waechter.simon at gmail.com
Mon Oct 13 14:52:34 UTC 2014



> On Oct. 13, 2014, 4:07 p.m., Milian Wolff wrote:
> > generatepage.cpp, line 85
> > <https://git.reviewboard.kde.org/r/120571/diff/1/?file=318212#file318212line85>
> >
> >     Directory "%1" could not be created.

Instead of directory path is used (newdest is an absolute path)


> On Oct. 13, 2014, 4:07 p.m., Milian Wolff wrote:
> > generatepage.cpp, line 221
> > <https://git.reviewboard.kde.org/r/120571/diff/1/?file=318212#file318212line221>
> >
> >     why that change? can't you have .zip archives on linux and tar archives on windows? this looks wrong

Replaced with the old raw-pointer. I will replace it with a QScopedPointer when I move the code to a shared library.


> On Oct. 13, 2014, 4:07 p.m., Milian Wolff wrote:
> > kapptemplate.cpp, line 78
> > <https://git.reviewboard.kde.org/r/120571/diff/1/?file=318214#file318214line78>
> >
> >     that looks wrong, is that really required? don't use c-style casts then

As I said in IRC: registerField() uses an UI element and a property (In this case the QLineEdit and the text() method of the QLineEdit). To register an element, the element needs a QWidget as base class - KUrlRequester and KLineEdit don't have one - the downcast from KLineEdit to QLineEdit makes it possible to use the text as property. Do you have a smoother solution for that ? For more information see: http://qt-project.org/doc/qt-5/qwizardpage.html#registerField


- Simon


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


On Oct. 13, 2014, 3:54 p.m., Simon Wächter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120571/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2014, 3:54 p.m.)
> 
> 
> Review request for KDevelop and Jonathan Riddell.
> 
> 
> Repository: kapptemplate
> 
> 
> Description
> -------
> 
> This patch drops the KDE4 library dependency:
> 
> - Removed the KDE4 library and 3 other CMake modules that where triggered by the support library
> - Fixed the includes and removed the old ones
> - Switched from kDebug to qCDebug
> - Rewritten copy and macro expanding
> - Removed a (unused) signal that tries to override the same signal in the base class (Results in warning)
> - Removed whitespaces
> 
> Please check:
> - Take a look at generatepage.cpp --> unpackArchive and generatepage.cpp --> finishFile. Is it a good idea to extract all macros in a file - even when it is a "binary blob" like an ODF or binary etc. ? Does someone know a good way to solve this ? In the end I always get stuck in a static file ending or mime type check (But there are some many minetypes ...)
> 
> Future:
> - Move the functionality inside a library, so that KAppTemplate and later KDevelop can use it to find file and project templates
> - Readd input checking patterns to the forms (Version, email etc)
> 
> 
> Diffs
> -----
> 
>   choicepage.cpp 7ae478f 
>   generatepage.h 22085d1 
>   generatepage.cpp 66b98ab 
>   kapptemplate.h 77f43c0 
>   kapptemplate.cpp b1abcab 
>   logging.h PRE-CREATION 
>   main.cpp 038238e 
>   properties.ui b1fac73 
>   CMakeLists.txt 4ea08d4 
>   apptemplatesmodel.cpp 2ef00d7 
> 
> Diff: https://git.reviewboard.kde.org/r/120571/diff/
> 
> 
> Testing
> -------
> 
> Testing was done under a project neon 5 system
> 
> 
> Thanks,
> 
> Simon Wächter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20141013/720b2476/attachment.html>


More information about the KDevelop-devel mailing list