<table><tr><td style="">apol requested changes to this revision.<br />apol added inline comments.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D25604">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D25604#inline-144439">View Inline</a><span style="color: #4b4d51; font-weight: bold;">startplasma.cpp:188</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    <span style="color: #74777d">// <span class="bright">Make sure that</span> the <span class="bright">KDE prefix is first</span> in <span class="bright">XDG_DATA_DIRS and that it's set at all.</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">    <span style="color: #74777d">// <span class="bright">The spec allows </span>XDG_DATA_DIRS <span class="bright">to be not set, but X session startup scripts tend</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #74777d">// <span class="bright">If</span> the <span class="bright">kde installation is</span> in <span class="bright">a non-standard location (ie. with a dev build),</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #74777d">// <span class="bright">it is possible the data root is not in the </span>XDG_DATA_DIRS <span class="bright">variable yet.</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please reduce the amount of comments.<br />
It's hard to read and not very descriptive. We can read the code as well. Instead of writing what it does, explain why if you really think it's necessary. At this point I'd be happy removing the comments overall.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D25604#inline-144440">View Inline</a><span style="color: #4b4d51; font-weight: bold;">startplasma.cpp:208</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">QString</span> <span class="n">kdeInstallFullDataRootDir</span> <span style="color: #aa2211">=</span> <span class="n">QStringLiteral</span><span class="p">(</span><span class="n">KDE_INSTALL_FULL_DATAROOTDIR</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span class="n">QStringList</span> <span class="n">xdgDataDirs</span> <span style="color: #aa2211">=</span> <span class="n">xdgDataDirsValue</span><span class="p">.</span><span class="n">split</span><span class="p">(</span><span class="n">QLatin1Char</span><span class="p">(</span><span style="color: #766510">':'</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">xdgDataDirs</span><span class="p">.</span><span class="n">contains</span><span class="p">(</span><span class="n">kdeInstallFullDataRootDir</span><span class="p">))</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Can do with splitRef.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R120 Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D25604">https://phabricator.kde.org/D25604</a></div></div><br /><div><strong>To: </strong>sniperrifle, apol<br /><strong>Cc: </strong>apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart<br /></div>