<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/120135/">https://git.reviewboard.kde.org/r/120135/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Some small nitpicks, otherwise looks good to me - thanks!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I also added some notes on what you could do in the future here.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">many thanks for taking up this project.</p></pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120135/diff/2/?file=311522#file311522line1" style="color: black; font-weight: bold; text-decoration: underline;">TODO</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">This is the current TODO list for the framework branch</pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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...</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120135/diff/2/?file=311525#file311525line50" style="color: black; font-weight: bold; text-decoration: underline;">apptemplatesmodel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">43</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">QString</span> <span class="n">localDescriptionsDir</span> <span class="o">=</span> <span class="n"><span class="hl">dirs</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">sav</span>eLocation</span><span class="p">(</span><span class="s"><span class="hl">"</span>apptemplate_description<span class="hl">s"</span></span><span class="p"><span class="hl">)</span>;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">50</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">QString</span> <span class="n">localDescriptionsDir</span> <span class="o">=</span> <span class="n"><span class="hl">QStandardPaths</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">writabl</span>eLocation</span><span class="p">(</span><span class="n"><span class="hl">QStandardPaths</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">GenericDataLocation</span></span><span class="p"><span class="hl">)</span></span><span class="hl"> </span><span class="o"><span class="hl">+</span></span><span class="hl"> </span><span class="s"><span class="hl">"/kdev</span>app<span class="hl">wizard/</span>template_description<span class="hl">/"</span></span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">another item for your TODO: use a QStringLiteral here and everywhere else you use string literals</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">but do that in a future patch, no need to complicate this patch with it now.</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120135/diff/2/?file=311525#file311525line91" style="color: black; font-weight: bold; text-decoration: underline;">apptemplatesmodel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void extractTemplateDescriptions( KStandardDirs* dirs )</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void extractTemplateDescriptions()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">90</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">QString</span> <span class="n">localDescriptionsDir</span> <span class="o">=</span> <span class="n">QStandardPaths</span><span class="o">::</span><span class="n">writableLocation</span><span class="p">(</span><span class="n">QStandardPaths</span><span class="o">::</span><span class="n">GenericDataLocation</span><span class="p">)</span> <span class="o">+</span> <span class="s">"/kdevappwizard/template_description/"</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">this repeats the code from above, you should share the code in a static free function</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120135/diff/2/?file=311526#file311526line79" style="color: black; font-weight: bold; text-decoration: underline;">choicepage.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void ChoicePage::saveConfig()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">76</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">picPath</span> <span class="o">=</span> <span class="n">dirs</span><span class="o">-></span><span class="n">findResource</span><span class="p">(</span><span class="s">"apptemplate_previews"</span><span class="p">,</span> <span class="n">index</span><span class="p">.</span><span class="n">data</span><span class="p">(</span><span class="n">Qt</span><span class="o">::</span><span class="n">UserRole</span><span class="o">+</span><span class="mi">2</span><span class="p">).</span><span class="n">toString</span><span class="p">());</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">79</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">foreach</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">templatePath</span><span class="p">,</span> <span class="n">templatePaths</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">when you cleanup the code style, here and elsewhere, you should add a space after foreach, if, else and the like.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">foreach (...)
</pre></div>
</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120135/diff/2/?file=311529#file311529line212" style="color: black; font-weight: bold; text-decoration: underline;">generatepage.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void GeneratePage::initializePage()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">193</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KArchive</span><span class="o">*</span> <span class="n">arch</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">212</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KArchive</span><span class="o">*</span> <span class="n">arch</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">future cleanup: use a scoped pointer.</p></pre>
</div>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/120135/diff/2/?file=311546#file311546line29" style="color: black; font-weight: bold; text-decoration: underline;">main.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">29</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">static</span> <span class="k">const</span> <span class="kt">char</span> <span class="n">description</span><span class="p">[]</span> <span class="o">=</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">29</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">static</span> <span class="k">const</span> <span class="kt">char</span> <span class="n">description</span><span class="p">[]</span> <span class="o">=</span> <span class="n">I18N_NOOP</span><span class="p">(</span><span class="s">"KAppTemplate is a KDE 4 project template generator"</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">thats not required anymore, afaik. just use i18n("...") below - this is now possible since you initialize the QApplication before</p></pre>
</div>
</div>
<br />
<p>- Milian Wolff</p>
<br />
<p>On September 11th, 2014, 10:39 a.m. UTC, Simon Wächter wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDevelop and Jonathan Riddell.</div>
<div>By Simon Wächter.</div>
<p style="color: grey;"><i>Updated Sept. 11, 2014, 10:39 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kapptemplate
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is an initial port of the KAppTemplate application to KF5. These things were changed:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Updated all CMake files</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Ported the codebased including the test cases (Everything still depends on KDELibs4Support)</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Changed the icon names, so they are installed in the right direction</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Opinions about that ?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If people accept the patch I will create a KF5 branch.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Compilation, testing and installing was done under a project neon 5 system. The core functionallity is working fine (For problems see the TODO file)</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>.gitignore <span style="color: grey">(PRE-CREATION)</span></li>
<li>CMakeLists.txt <span style="color: grey">(0c162c4)</span></li>
<li>TODO <span style="color: grey">(PRE-CREATION)</span></li>
<li>apptemplateitem.cpp <span style="color: grey">(01bbc5f)</span></li>
<li>apptemplatesmodel.h <span style="color: grey">(c54994a)</span></li>
<li>apptemplatesmodel.cpp <span style="color: grey">(5b62bbd)</span></li>
<li>choicepage.cpp <span style="color: grey">(aa4bf9c)</span></li>
<li>cmake/modules/KAppTemplateMacro.cmake <span style="color: grey">(1dd61b2)</span></li>
<li>doc/CMakeLists.txt <span style="color: grey">(f9ca3cc)</span></li>
<li>generatepage.cpp <span style="color: grey">(fde5c75)</span></li>
<li>icons/CMakeLists.txt <span style="color: grey">(69b754f)</span></li>
<li>icons/hi128-app-kapptemplate.png <span style="color: grey">(fed1062)</span></li>
<li>icons/hi128-apps-kapptemplate.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>icons/hi16-app-kapptemplate.png <span style="color: grey">(aba9f96)</span></li>
<li>icons/hi16-apps-kapptemplate.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>icons/hi22-app-kapptemplate.png <span style="color: grey">(2205f64)</span></li>
<li>icons/hi22-apps-kapptemplate.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>icons/hi32-app-kapptemplate.png <span style="color: grey">(2c87092)</span></li>
<li>icons/hi32-apps-kapptemplate.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>icons/hi48-app-kapptemplate.png <span style="color: grey">(5e896b9)</span></li>
<li>icons/hi48-apps-kapptemplate.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>icons/hi64-app-kapptemplate.png <span style="color: grey">(4f4062d)</span></li>
<li>icons/hi64-apps-kapptemplate.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>icons/hisc-app-kapptemplate.svg <span style="color: grey">(e7d38fa)</span></li>
<li>icons/hisc-apps-kapptemplate.svg <span style="color: grey">(PRE-CREATION)</span></li>
<li>kapptemplate.cpp <span style="color: grey">(a221ac4)</span></li>
<li>main.cpp <span style="color: grey">(068e714)</span></li>
<li>tests/CMakeLists.txt <span style="color: grey">(2a70758)</span></li>
<li>tests/macrosubstitutiontest.cpp <span style="color: grey">(8a35536)</span></li>
<li>tests/namevalidatortest.cpp <span style="color: grey">(e75e28a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120135/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>