<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/121775/">https://git.reviewboard.kde.org/r/121775/</a>
</td>
</tr>
</table>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I wonder why I never saw this issue. Can you explain what you are encountering? Also, adding a unit test would help here to ensure we never run into this issue again.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Generally, the code for checkForDependencies ensures that all dependencies are available and none are disabled. Only then should potential dependencies be loaded. Your code now always tries to load dependencies, no? Even when one of them is disabled or not existing.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">so -1 from my side until you give more information on what issue you are encountering and trying to solve.</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/121775/diff/1/?file=337592#file337592line517" style="color: black; font-weight: bold; text-decoration: underline;">shell/plugincontroller.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">IPlugin *PluginController::loadPluginInternal( const QString &pluginId )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">516</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="o">!</span><span class="n">loadDependencies</span><span class="p">(</span> <span class="n">info</span><span class="p">,</span> <span class="n">failedDependency</span> <span class="p">)</span> <span class="p">)</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">513</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="o">!</span><span class="n">loadDependencies</span><span class="p">(</span> <span class="n">info</span><span class="p">,</span> <span class="n">failedDependency</span> <span class="p">)</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;">pass the missing dependencies here, otherwise you don't need the dependency list here at all</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/121775/diff/1/?file=337592#file337592line586" style="color: black; font-weight: bold; text-decoration: underline;">shell/plugincontroller.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">bool PluginController::checkForDependencies( const KPluginInfo& info, QStringList& missing ) const</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QStringList PluginController::dependenciesForPlugin( const KPluginInfo& pluginInfo ) const</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">568</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt"><span class="hl">bool</span></span> <span class="n">PluginController</span><span class="o">::</span><span class="n"><span class="hl">checkForD</span>ependencies</span><span class="p">(</span> <span class="k">const</span> <span class="n">KPluginInfo</span><span class="o">&</span> <span class="n"><span class="hl">i</span>nfo</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">QStringList</span></span><span class="o"><span class="hl">&</span></span><span class="hl"> </span><span class="n"><span class="hl">missing</span></span> <span class="p">)</span> <span class="k">const</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">565</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n"><span class="hl">QStringList</span></span> <span class="n">PluginController</span><span class="o">::</span><span class="n"><span class="hl">d</span>ependencies<span class="hl">ForPlugin</span></span><span class="p">(</span> <span class="k">const</span> <span class="n">KPluginInfo</span><span class="o">&</span> <span class="n"><span class="hl">pluginI</span>nfo</span> <span class="p">)</span> <span class="k">const</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;">just return the QSet<QString>, no? why converting it to a list? Or just return a bool and rename this to "hasMissingDependencies" or such?</p></pre>
</div>
</div>
<br />
<p>- Milian Wolff</p>
<br />
<p>On December 31st, 2014, 12:34 p.m. UTC, Sergey Kalinichev 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.</div>
<div>By Sergey Kalinichev.</div>
<p style="color: grey;"><i>Updated Dec. 31, 2014, 12:34 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</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;">Currently if some plugin depends on another one that's not yet loaded, the former one won't be loaded too.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This happens because checkForDependencies returns false if there are some dependencies, but the code that calls this method threats false return code as an error, so it doesn't try to load any dependencies at all.</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>shell/plugincontroller.h <span style="color: grey">(89b0b8b)</span></li>
<li>shell/plugincontroller.cpp <span style="color: grey">(f70fc64)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121775/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>