<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/109315/">http://git.reviewboard.kde.org/r/109315/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 6th, 2013, 9:29 p.m. UTC, <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/109315/diff/2/?file=117411#file117411line12" style="color: black; font-weight: bold; text-decoration: underline;">CMakeLists.txt</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">12</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nb">set</span><span class="p">(</span><span class="s">KDEV_PLUGIN_VERSION</span> <span class="s">16</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This will have to be included in the KDevPlatform config module so that plugins outside of kdevplatform can use it.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">agreed, I planned to do that in a next patch.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 6th, 2013, 9:29 p.m. UTC, <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/109315/diff/2/?file=117413#file117413line45" style="color: black; font-weight: bold; text-decoration: underline;">interfaces/iplugin.h</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; ">class QAction;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">45</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#define KDEVELOP_PLUGIN_VERSION 16</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is there a particular reason for breaking the source-compatibility here? You could just #include "ipluginversion.h" here and drop the change in iplugincontroller.cpp and have any code that actually uses the define keep on working.
Or do you want users to explicitly include the version-header and catch them this way?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, basically nobody is using this header besides the I/PluginController, I'm actually unsure if we should expose this number at all (the file is being installed, here).
But no, there's no reason. I can put the include if you think that maintaining this would be benefitial.</pre>
<br />
<p>- Aleix</p>
<br />
<p>On March 6th, 2013, 4:55 p.m. UTC, Aleix Pol Gonzalez wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop, Andreas Pakulat and Milian Wolff.</div>
<div>By Aleix Pol Gonzalez.</div>
<p style="color: grey;"><i>Updated March 6, 2013, 4:55 p.m.</i></p>
<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;">One of the worst things when developing plugins in kdev is that we need to target an ABI version number. It makes sense to have it but it is possible to have an API-compatible plugin that isn't compatible due to the ABI version.
Also, every time we change kdevplatform version, we have to go through all plugins an change the version number in all projects.
In this patch, I'm using cmake to configure all the desktop files in runtime. If you guys like the idea, I'll implement it for KDevelop.
There are other things we could do, since we're generating it, we could set the KDE-PluginInfo-Version to kdevplatform's, I think it would make sense.
In case somebody wonders, scripty already knows about .desktop.cmake files.
For reference, I used these bash commands for changing it to the new way:
find . -name kdev*.desktop | xargs -I @ git mv @ @.cmake
find . -name *.desktop.cmake | xargs sed -i -e 's/X-KDevelop-Version=16/X-KDevelop-Version=@KDEV_PLUGIN_VERSION@/g'
find . -name CMakeLists.txt | xargs sed -r -i -e "s/install\( *FILES kdev(.*).desktop/configure_file(kdev\1.desktop.cmake \${CMAKE_CURRENT_BINARY_DIR}\/kdev\1.desktop)\ninstall(FILES \${CMAKE_CURRENT_BINARY_DIR}\/kdev\1.desktop/g"</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;">The installed desktop files look the same as they used to.</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>CMakeLists.txt <span style="color: grey">(d0bcfa7)</span></li>
<li>interfaces/CMakeLists.txt <span style="color: grey">(5e83497)</span></li>
<li>interfaces/iplugin.h <span style="color: grey">(3a30eda)</span></li>
<li>interfaces/iplugincontroller.cpp <span style="color: grey">(d7e8764)</span></li>
<li>plugins/appwizard/CMakeLists.txt <span style="color: grey">(26b0f4c)</span></li>
<li>plugins/appwizard/kdevappwizard.desktop.cmake <span style="color: grey">(2ffcb12)</span></li>
<li>plugins/classbrowser/CMakeLists.txt <span style="color: grey">(bfb1688)</span></li>
<li>plugins/classbrowser/kdevclassbrowser.desktop.cmake <span style="color: grey">(5983d4b)</span></li>
<li>plugins/codeutils/CMakeLists.txt <span style="color: grey">(3694060)</span></li>
<li>plugins/codeutils/kdevcodeutils.desktop.cmake <span style="color: grey">(70a6b88)</span></li>
<li>plugins/contextbrowser/CMakeLists.txt <span style="color: grey">(abf573a)</span></li>
<li>plugins/contextbrowser/kdevcontextbrowser.desktop.cmake <span style="color: grey">(b8a0386)</span></li>
<li>plugins/cvs/CMakeLists.txt <span style="color: grey">(150ff18)</span></li>
<li>plugins/cvs/kdevcvs.desktop.cmake <span style="color: grey">(dd7939c)</span></li>
<li>plugins/dashboard/CMakeLists.txt <span style="color: grey">(3ed5574)</span></li>
<li>plugins/dashboard/kdevprojectdashboard.desktop.cmake <span style="color: grey">(28bf473)</span></li>
<li>plugins/dashboard/plasmoids/projectfileplasmoid/CMakeLists.txt <span style="color: grey">(d3016d7)</span></li>
<li>plugins/documentswitcher/CMakeLists.txt <span style="color: grey">(9312337)</span></li>
<li>plugins/documentswitcher/kdevdocumentswitcher.desktop.cmake <span style="color: grey">(e45a9ef)</span></li>
<li>plugins/documentview/CMakeLists.txt <span style="color: grey">(30fc272)</span></li>
<li>plugins/documentview/kdevdocumentview.desktop.cmake <span style="color: grey">(5187c81)</span></li>
<li>plugins/execute/CMakeLists.txt <span style="color: grey">(6c65936)</span></li>
<li>plugins/execute/kdevexecute.desktop.cmake <span style="color: grey">(a71f502)</span></li>
<li>plugins/executescript/CMakeLists.txt <span style="color: grey">(2b90327)</span></li>
<li>plugins/executescript/kdevexecutescript.desktop.cmake <span style="color: grey">(83fd95d)</span></li>
<li>plugins/externalscript/CMakeLists.txt <span style="color: grey">(c8f2f73)</span></li>
<li>plugins/externalscript/kdevexternalscript.desktop.cmake <span style="color: grey">(cd97a2f)</span></li>
<li>plugins/filemanager/CMakeLists.txt <span style="color: grey">(a6a6a9f)</span></li>
<li>plugins/filemanager/kdevfilemanager.desktop.cmake <span style="color: grey">(6b94c5d)</span></li>
<li>plugins/filetemplates/CMakeLists.txt <span style="color: grey">(77b3be7)</span></li>
<li>plugins/filetemplates/kdevfiletemplates.desktop.cmake <span style="color: grey">(7d1434f)</span></li>
<li>plugins/genericprojectmanager/CMakeLists.txt <span style="color: grey">(b081a1b)</span></li>
<li>plugins/genericprojectmanager/kdevgenericmanager.desktop.cmake <span style="color: grey">(8712aab)</span></li>
<li>plugins/git/CMakeLists.txt <span style="color: grey">(f2bf0f1)</span></li>
<li>plugins/git/kdevgit.desktop.cmake <span style="color: grey">(e1aceff)</span></li>
<li>plugins/grepview/CMakeLists.txt <span style="color: grey">(f9e87af)</span></li>
<li>plugins/grepview/kdevgrepview.desktop.cmake <span style="color: grey">(ec085ef)</span></li>
<li>plugins/konsole/CMakeLists.txt <span style="color: grey">(034b4a2)</span></li>
<li>plugins/konsole/kdevkonsoleview.desktop.cmake <span style="color: grey">(1778364)</span></li>
<li>plugins/openwith/CMakeLists.txt <span style="color: grey">(3217702)</span></li>
<li>plugins/openwith/kdevopenwith.desktop.cmake <span style="color: grey">(b484570)</span></li>
<li>plugins/pastebin/CMakeLists.txt <span style="color: grey">(c9fe952)</span></li>
<li>plugins/pastebin/kdevpastebin.desktop.cmake <span style="color: grey">(a25f638)</span></li>
<li>plugins/patchreview/CMakeLists.txt <span style="color: grey">(8bedb83)</span></li>
<li>plugins/patchreview/kdevpatchreview.desktop.cmake <span style="color: grey">(3380d21)</span></li>
<li>plugins/problemreporter/CMakeLists.txt <span style="color: grey">(89b197d)</span></li>
<li>plugins/problemreporter/kdevproblemreporter.desktop.cmake <span style="color: grey">(c20f0d2)</span></li>
<li>plugins/projectmanagerview/CMakeLists.txt <span style="color: grey">(aec8360)</span></li>
<li>plugins/projectmanagerview/kdevprojectmanagerview.desktop.cmake <span style="color: grey">(542e088)</span></li>
<li>plugins/quickopen/CMakeLists.txt <span style="color: grey">(b2b3f30)</span></li>
<li>plugins/quickopen/kdevquickopen.desktop.cmake <span style="color: grey">(8a4be70)</span></li>
<li>plugins/reviewboard/CMakeLists.txt <span style="color: grey">(89892f7)</span></li>
<li>plugins/reviewboard/kdevreviewboard.desktop.cmake <span style="color: grey">(7a83544)</span></li>
<li>plugins/snippet/CMakeLists.txt <span style="color: grey">(04433b5)</span></li>
<li>plugins/snippet/kdevsnippet.desktop.cmake <span style="color: grey">(5d2002a)</span></li>
<li>plugins/standardoutputview/CMakeLists.txt <span style="color: grey">(ba8d434)</span></li>
<li>plugins/standardoutputview/kdevstandardoutputview.desktop.cmake <span style="color: grey">(8331422)</span></li>
<li>plugins/subversion/CMakeLists.txt <span style="color: grey">(f74a45f)</span></li>
<li>plugins/subversion/kdevsubversion.desktop.cmake <span style="color: grey">(e63068f)</span></li>
<li>plugins/switchtobuddy/CMakeLists.txt <span style="color: grey">(795b5ed)</span></li>
<li>plugins/switchtobuddy/kdevswitchtobuddy.desktop.cmake <span style="color: grey">(7d5f59f)</span></li>
<li>plugins/templatemanager/CMakeLists.txt <span style="color: grey">(cc3ea92)</span></li>
<li>plugins/testview/CMakeLists.txt <span style="color: grey">(5307d7f)</span></li>
<li>plugins/testview/kdevtestview.desktop.cmake <span style="color: grey">(5980536)</span></li>
<li>plugins/vcschangesview/CMakeLists.txt <span style="color: grey">(a284b28)</span></li>
<li>plugins/vcschangesview/kdevvcschangesview.desktop.cmake <span style="color: grey">(41646c6)</span></li>
<li>shell/kross/kdevkrossplugin.desktop.cmake <span style="color: grey">(ddb8e9c)</span></li>
<li>shell/plugincontroller.cpp <span style="color: grey">(009228b)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109315/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>