<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 />
<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="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="margin-left: 2em; 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>
</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="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="margin-left: 2em; 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>
</div>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Apart from those two minor points I think this is safe and ok to do. If a plugin is being built against a particular plugin-version-number and later loaded into a newer kdevplatform-app it won't be loaded. If its rebuilt and still builds (i.e. no source-incompatible changes) its safe to auto-update it to the current plugin-version.
The only real issue that I can see happening is when the API did not change, but the behaviour changed. That case is handled a very small bit better right now, as a simple recompile won't load plugins that haven't been used for months/years against newer kdevplatform. At the same time its a relatively weak point since thorough testing of a plugin against a newer kdevplatform version before pushing a version-number update out happens very seldomly (if at all). Most of the time people just bump the version and if it compiles consider everything to still work.</pre>
<p>- Andreas</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>